dbus2 / zbus-old

Rust D-Bus crate.
https://gitlab.freedesktop.org/dbus/zbus
Other
49 stars 13 forks source link

enum to string in function signature? #255

Closed zeenix closed 1 year ago

zeenix commented 2 years ago

In GitLab by @fluke on Jan 18, 2022, 01:23

I don't know if this is an issue, more likely It's that I'm unsure of how best to do this.

The following is everything reduced down to an example:

#[dbus_proxy(
    interface = "org.freedesktop.login1.Manager",
    default_service = "org.freedesktop.login1",
    default_path = "/org/freedesktop/login1"
)]
trait Manager {
//...
    /// Inhibit method
    #[inline]
    fn inhibit(
        &self,
        what: InhibitThis, // Problem point, this is `s`
        who: &str,
        why: &str,
        mode: &str,
    ) -> zbus::Result<std::os::unix::io::RawFd>;
}
#[derive(Debug, PartialEq, Clone, Serialize, Deserialize)]
pub enum InhibitThis {
    Shutdown,
    Sleep,
    Idle,
    HandlePowerKey,
    HandleSuspendKey,
    HandleHibernateKey,
    HandleLidSwitch,
    Invalid,
}

impl From<&str> for InhibitThis {
    fn from(s: &str) -> Self {
        match s.trim() {
            "shutdown" => Self::Shutdown,
            "sleep" => Self::Sleep,
            "idle" => Self::Idle,
            "handle-power-key" => Self::HandlePowerKey,
            "handle-suspend-key" => Self::HandleSuspendKey,
            "handle-hibernate-key" => Self::HandleHibernateKey,
            "handle-lid-switch" => Self::HandleLidSwitch,
            _ => Self::Invalid,
        }
    }
}

impl From<InhibitThis> for &str {
    fn from(s: InhibitThis) -> Self {
        match s {
            InhibitThis::Shutdown => "shutdown",
            InhibitThis::Sleep => "sleep",
            InhibitThis::Idle => "idle",
            InhibitThis::HandlePowerKey => "handle-power-key",
            InhibitThis::HandleSuspendKey => "handle-suspend-key",
            InhibitThis::HandleHibernateKey => "handle-hibernate-key",
            InhibitThis::HandleLidSwitch => "handle-lid-switch",
            InhibitThis::Invalid => "invalid",
        }
    }
}

impl TryFrom<OwnedValue> for InhibitThis {
    type Error = zbus::Error;

    fn try_from(value: OwnedValue) -> Result<Self, Self::Error> {
        let value = <String>::try_from(value)?;
        return Ok(Self::from(value.as_str()));
    }
}

impl TryFrom<InhibitThis> for Value<'_> {
    type Error = zbus::Error;

    fn try_from(value: InhibitThis) -> Result<Self, Self::Error> {
        Ok(Value::Str(zvariant::Str::from(<&str>::from(value))))
    }
}

impl TryFrom<InhibitThis> for OwnedValue {
    type Error = zbus::Error;

    fn try_from(value: InhibitThis) -> Result<Self, Self::Error> {
        Ok(OwnedValue::from(zvariant::Str::from(<&str>::from(value))))
    }
}

impl Type for InhibitThis {
    fn signature() -> zvariant::Signature<'static> {
        Signature::from_str_unchecked("s")
    }
}

I'm sort of jumping around trying to find the solution here. InhibitThis needs to be s dbus type, and the derives usually turn in to u. I imagine I need to manually ser/de?

I also tried:

    /// Inhibit method
    #[inline]
    fn inhibit(
        &self,
        data: SomeStruct,
    ) -> zbus::Result<std::os::unix::io::RawFd>;
}

but the signature ends up being (ssss) when ssss is required. There's a lot of cases like this in logind, so any tips would be greatly appreciated.

zeenix commented 2 years ago

I imagine I need to manually ser/de?

I think you probably just need the untagged attribute. Also with latest zvariant release, you can use Type derive macro instead of the manual impl:

#[derive(Debug, PartialEq, Clone, Serialize, Deserialize, Type)]
#[serde(untagged)]
#[zvariant(signature = "s")]
pub enum InhibitThis {
    Shutdown,
    Sleep,
    Idle,
    HandlePowerKey,
    HandleSuspendKey,
    HandleHibernateKey,
    HandleLidSwitch,
    Invalid,
}

Regarding the manual Value conversions (which btw, you only need if you use this type as a D-Bus property) are currently needed unfortunately but it shouldn't be hard to make Value and OwnedValue derive macros to do what you need by honoring all basic signature types passed through the signature attribute. It already does the right thing when it's set to a{sv} or dict.

zeenix commented 2 years ago

hmm.. you may need serde-repr if that doesn't work.

zeenix commented 2 years ago

I hope I answered your question(?). I'll re-use this issue for:

it shouldn't be hard to make Value and OwnedValue derive macros to do what you need by honoring all basic signature types passed through the signature attribute. It already does the right thing when it's set to a{sv} or dict.

zeenix commented 2 years ago

In GitLab by @fluke on Jan 26, 2022, 13:15

Sorry about the delay. Just so busy.

I tried #[zvariant(signature = "s")] but it's not an attribute? Additionally I was manually implementing Type due to not getting the correct type I needed (in this case s with to/from String).

It seems serde-repr is for integer serde of enum only?

My issue might be best described as "The function argument is a &str and I need to limit the actual &str content with an enum + to/from String". Serde seems to be failing in combination with zbus where I have the Type as s, but serde is still serializing to integer.

I haven't done custom serde trait implementation in a while but I'll have a go at that later.

Versions in use:

serde = "^1.0"
zbus = "^2.0.0"
zvariant = "^3.1.0"
zeenix commented 2 years ago

In GitLab by @fluke on Jan 26, 2022, 13:45

Well that was a lot more trivial than I thought it would be:

impl Serialize for InhibitThis {
    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
    where
        S: serde::Serializer {
        serializer.serialize_str(self.into())
    }
}

impl<'de> Deserialize<'de> for InhibitThis {
    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
    where
        D: serde::Deserializer<'de> {
        let s = String::deserialize(deserializer)?;
        Ok(InhibitThis::from(s.as_str()))
    }
}

impl Type for InhibitThis {
    fn signature() -> zvariant::Signature<'static> {
        Signature::from_str_unchecked("s")
    }
}

in conjunction with the various from traits does what I need it to. I don't really understand why the base derives weren't producing this though (with also using #[serde(rename(serialize = "ser_name", deserialize = "de_name"))] attributes)

zeenix commented 2 years ago

I tried #[zvariant(signature = "s")] but it's not an attribute?

Yes, it is. What's the code you tried and what is the error you get?

It seems serde-repr is for integer serde of enum only?

hmm.. maybe. I just confused by this comment.

My issue might be best described as "The function argument is a &str and I need to limit the actual &str content with an enum + to/from String". Serde seems to be failing in combination with zbus where I have the Type as s, but serde is still serializing to integer.

Actually it's zvariant's serializer/deserializer that are choosing integer representations for unit variants of enums. That is typically what you'd want for efficiency. I do see the value of your use case though and I'm glad to hear that you found a not-so-hard solution after all.

zeenix commented 2 years ago

I tried #[zvariant(signature = "s")] but it's not an attribute?

Yes, it is. What's the code you tried and what is the error you get?

Just to be clear, that's an attribute used by Type derive macro so you'll need that.

zeenix commented 2 years ago

I tried #[zvariant(signature = "s")] but it's not an attribute?

Yes, it is. What's the code you tried and what is the error you get?

ping?

zeenix commented 2 years ago

with also using #[serde(rename(serialize = "ser_name", deserialize = "de_name"))] attributes

Those are just for renaming the fields/enum, so not sure why you'd expect that to help.

zeenix commented 2 years ago

In GitLab by @fluke on Jan 27, 2022, 22:16

Because I'm not familiar with zbus in the same capacity you might be so I was reaching for things I had some knowledge of.

zeenix commented 2 years ago

In GitLab by @fluke on Jan 27, 2022, 22:28

As per the examples above, with:

// ignore unused imports
use serde::{Deserialize, Serialize};
use zbus::fdo;
use zvariant::{OwnedObjectPath, OwnedValue, Signature, Structure, Type};

#[derive(Debug, PartialEq, Clone, Serialize, Deserialize, Type)]
#[zvariant(signature = "s")]
pub enum InhibitThis {
    Shutdown,
    Sleep,
    Idle,
    HandlePowerKey,
    HandleSuspendKey,
    HandleHibernateKey,
    HandleLidSwitch,
    Invalid,
}

error:

cannot find attribute `zvariant` in this scope

note: `zvariant` is in scope, but it is a crate, not an attribute
zeenix commented 2 years ago

Yup, I can reproduce. You sir, found a bug. :smile: Our tests only use it with SerializeDict and DeserializeDict and not Type alone so it didn't get caught. I'll try and release a fix ASAP.

zeenix commented 2 years ago

That's not zbus or zvariant but serde. I just thought maybe you know something that I don't.

zeenix commented 2 years ago

In GitLab by @fluke on Jan 28, 2022, 24:08

Ah I see. I suspect once you fix the bug I unwittingly found then maybe #[serde(rename might actually be very useful here :smile:

zeenix commented 2 years ago

Turned out to be a simple mistake (!461).

zeenix commented 2 years ago

mentioned in commit ebe21609e15bab367e1dff23fb60501ab1f8eb65

zeenix commented 2 years ago

Fix released. cargo update to get it. :smile:

zeenix commented 2 years ago

In GitLab by @fluke on Jan 28, 2022, 07:38

Excellent, thanks.

Okay so the end result is I still need manual serde for use of enum as an s arg.

#[derive(Debug, PartialEq, Copy, Clone, Type)]
#[zvariant(signature = "s")]
pub enum InhibitType {
    Shutdown,
    Sleep,
    Idle,
    HandlePowerKey,
    HandleSuspendKey,
    HandleHibernateKey,
    HandleLidSwitch,
}

impl FromStr for InhibitType {
    type Err = fdo::Error;

    fn from_str(s: &str) -> Result<Self, Self::Err> {
        let res = match s.trim() {
            "shutdown" => Self::Shutdown,
            "sleep" => Self::Sleep,
            "idle" => Self::Idle,
            "handle-power-key" => Self::HandlePowerKey,
            "handle-suspend-key" => Self::HandleSuspendKey,
            "handle-hibernate-key" => Self::HandleHibernateKey,
            "handle-lid-switch" => Self::HandleLidSwitch,
            _ => return Err(fdo::Error::IOError(format!("{} is an invalid variant", s))),
        };
        Ok(res)
    }
}

impl From<InhibitType> for &str {
    fn from(s: InhibitType) -> Self {
        match s {
            InhibitType::Shutdown => "shutdown",
            InhibitType::Sleep => "sleep",
            InhibitType::Idle => "idle",
            InhibitType::HandlePowerKey => "handle-power-key",
            InhibitType::HandleSuspendKey => "handle-suspend-key",
            InhibitType::HandleHibernateKey => "handle-hibernate-key",
            InhibitType::HandleLidSwitch => "handle-lid-switch",
        }
    }
}

impl Serialize for InhibitType {
    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
        where
            S: serde::Serializer {
        serializer.serialize_str((*self).into())
    }
}

impl<'de> Deserialize<'de> for InhibitType {
    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
        where
            D: serde::Deserializer<'de> {
        let s = String::deserialize(deserializer)?;
        InhibitType::from_str(s.as_str()).map_err(serde::de::Error::custom)
    }
}

is the final thing. Do you want me to file this as a new issue? Is it something related to zbus at all? I think in most cases an enum -> int is the correct behaviour, but here I need zbus to send the variants as strings.

zeenix commented 2 years ago

is the final thing. Do you want me to file this as a new issue? Is it something related to zbus at all? I think in most cases an enum -> int is the correct behaviour, but here I need zbus to send the variants as strings.

I'm not sure zvariant can do much here. There can definitely be a macro that achieves this though, maybe a new serde_str crate is in order (or perhaps something like this already exists)? :smile: Not having to deal with all the serialization/deserialization issues was supposed to be one of the advantages of having a serde-based API.

What you could do is make use of the derive_more crate to make the From<InhibitType> for &str part easier at least. It also supports FromStr but doesn't support enums for that (perhaps that could be a feature request?). There is at least one crate that provides that but I don't know about the maturity or maintainance status of it.