instant-labs / instant-xml

11 stars 2 forks source link

Correct way to emit namespace prefix on a container? #59

Open wez opened 1 month ago

wez commented 1 month ago

Thank you for building this crate! I'm doing some stuff that unfortunately requires doing SOAP from some generated code. I'm currently looking at replacing this bit of manual xml generation with ToXml enabled types:

       let body = format!(
            r#"
            <s:Envelope xmlns:s="http://schemas.xmlsoap.org/soap/envelope/"
                s:encodingStyle="http://schemas.xmlsoap.org/soap/encoding/">
                <s:Body>
                    <u:{action} xmlns:u="{service}">
                        {payload}
                    </u:{action}>
                </s:Body>
            </s:Envelope>"#,

I'm having a difficult time figuring out the ns attribute functionality; I've been looking at the test cases to get a feel for it.

This is what I've come up with, which seems to emit the correct text, but I'm not sure that this is the correct way to do it:

#[derive(ToXml, Debug, Clone, PartialEq)]
#[xml(
    rename = "u:Stop",
    ns("", u = "urn:schemas-upnp-org:service:AVTransport:1")
)]
pub struct StopRequest {
    #[xml(rename = "InstanceID")]
    pub instance_id: u32,
}

const SOAP_ENCODING: &str = "http://schemas.xmlsoap.org/soap/encoding/";
const SOAP_ENVELOPE: &str = "http://schemas.xmlsoap.org/soap/envelope/";

#[derive(Debug, Eq, PartialEq, ToXml)]
#[xml(rename="s:Envelope", ns(
    "",
    s = SOAP_ENVELOPE
))]
struct Envelope<T: ToXml> {
    #[xml(attribute, rename = "s:encodingStyle")]
    encoding_style: &'static str,
    body: Body<T>,
}

#[derive(Debug, Eq, PartialEq, ToXml)]
#[xml(rename = "s:Body")]
struct Body<T: ToXml> {
    payload: T,
}

let action = Envelope {
    encoding_style: SOAP_ENCODING,
    body: Body {
        payload: StopRequest { instance_id: 0 },
    },
};

assert_eq!(
    instant_xml::to_string(&action).unwrap(),
    "<s:Envelope xmlns:s=\"http://schemas.xmlsoap.org/soap/envelope/\" \
        s:encodingStyle=\"http://schemas.xmlsoap.org/soap/encoding/\">\
        <s:Body>\
        <u:Stop xmlns:u=\"urn:schemas-upnp-org:service:AVTransport:1\">\
        <InstanceID>0</InstanceID>\
        </u:Stop>\
        </s:Body>\
        </s:Envelope>"
);

Is this "right"? Is there a better way? What do you recommend?

I want to note that it feels important for the outer Envelope to not have a default inherited namespace, because the inner payload contents (the InstanceID element) are supposed to not be in any particular namespace AFAICT.

I also want to note that it's been just shy of 20 years(!) since I last did anything in earnest with XML and I am foggy on the details of this stuff. I'm kinda hoping that I can skirt the edges of re-learning it, so please forgive me if I have some obvious gaps on how things work!

Thanks in advance!

djc commented 1 month ago

Hey, cool to see some external usage! Happy to gather some feedback on this project.

Is this "right"? Is there a better way? What do you recommend?

I would expect that you should not need s: prefixes in any of the rename attributes (and in fact, I'd be inclined to make that a compile-time error at least for rename values that are literals) -- the serializer should take care of adding those based on your ns attributes. (Perhaps you're coming here from quick-xml and trying to workaround its namespace handling?)

I want to note that it feels important for the outer Envelope to not have a default inherited namespace, because the inner payload contents (the InstanceID element) are supposed to not be in any particular namespace AFAICT.

It's definitely possible that this isn't well-supported today, but if you like we could hash out a strategy for how we might want to support it? Based on quickly skimming ns() usage in the test cases in order page some context back in:

instant-xml/tests/de-ns.rs
11:#[xml(ns("URI", bar = "BAZ"))]
13:    #[xml(ns("BAZ"))]
18:#[xml(ns("URI", bar = "BAZ"))]
24:#[xml(ns("URI", bar = "BAZ"))]
83:#[xml(ns("URI", bar = "BAZ"))]
85:    #[xml(ns("BAZ"))]
90:#[xml(ns("URI", bar = "BAZ"))]
163:#[xml(ns("URI", da_sh.ed-ns = "dashed"))]
165:    #[xml(ns("dashed"))]
170:fn dashed_ns() {

instant-xml/tests/ser-child-ns.rs
6:#[xml(ns(dar = "BAZ", internal = INTERNAL))]
8:    #[xml(ns(INTERNAL))]
13:#[xml(ns("URI", bar = "BAZ", foo = "BAR"))]
20:#[xml(ns("URI", dar = DAR, internal = INTERNAL))]
22:    #[xml(ns(DAR))]
24:    #[xml(ns(INTERNAL))]

instant-xml/tests/ser-nested.rs
6:#[xml(ns("URI", dar = "BAZ", internal = INTERNAL))]
8:    #[xml(ns(INTERNAL))]
15:#[xml(ns("URI", bar = "BAZ", foo = "BAR"))]
19:    #[xml(ns("BAZ"))]
21:    #[xml(ns("DIFFERENT"))]

instant-xml/tests/de-direct.rs
6:#[xml(ns("URI"))]
8:    #[xml(ns("BAZ"))]

instant-xml/tests/de-nested.rs
6:#[xml(ns("URI", bar = BAR))]
8:    #[xml(ns(BAR))]
13:#[xml(ns("URI", bar = "BAZ", foo = "BAR"))]
15:    #[xml(ns(BAR))]

instant-xml/tests/ser-attr-ns.rs
6:#[xml(ns(bar = "BAR"))]
8:    #[xml(attribute, ns(bar))]
13:fn no_prefix_attr_ns() {

instant-xml/tests/escaping.rs
8:#[xml(ns("URI"))]

instant-xml/tests/scalar.rs
8:#[xml(ns("URI"))]
15:#[xml(ns("URI"))]

instant-xml/tests/ser-named.rs
6:#[xml(ns(bar = "BAZ", foo = "BAR"))]
9:    #[xml(ns("BAZ"))]
11:    #[xml(ns("typo"))]

I think the unnamed namespace literal basically translates to an xmlns=".." attribute (see ContainerMeta::default_namespace()) while named meta key/value pairs translate to xmlns:key="value" attributes, so this might not quite be what you're looking for?

If you have counterexamples of Rust types that you think serialize (or deserialize) incorrectly, I can probably take a look (though patches are also, of course, very welcome!).

Like you, most of my XML experience is from a while back so I spent some time reading the spec to try to make this crate correct -- but my main goal was getting instant-epp working correctly so I haven't had much of a look at other uses.

(Also if you want to chat on Discord that's also an option.)

djc commented 1 month ago

Thinking more about this, the InstanceID example is interesting because IIRC I decided that, while types (in terms of procedural macros) don't know about each other and, as such, define their namespaces independently (and this should be correctly implemented in the serializer/deserializer) such that there's no (incorrect) inheritance of default namespaces, I felt that it would make sense for fields-as-elements to inherent the parent type's namespace by default. I do think we support specifying ns() on fields, too, though, so maybe what you want to here is to override the namespace on the field element?

wez commented 1 month ago

Thanks; I've gotten this working now. The rename wasn't good, and it seemingly (perhaps it was just a terse/misleading error message?) wouldn't match in deserialization in some circumstances. I didn't keep track of the problem case, sorry!

I've found and worked around a couple of issues; I'll try to trickle in some PRs for the things that I couldn't work around, and maybe file some issues for the things that I worked around and didn't find a way to just fix at the source easily.

djc commented 1 month ago

Sounds great, thanks!