Azure / azure-sdk-for-rust

This repository is for active development of the *unofficial* Azure SDK for Rust. This repository is *not* supported by the Azure SDK team.
MIT License
696 stars 241 forks source link

application/json-merge+patch support #1649

Open heaths opened 5 months ago

heaths commented 5 months ago

We need to figure out how to support application/json-merge+patch as described in our guidelines. We current plan to declare all fields as Option<T> to support an unfortunately all too common case of a service specification being incongruent with the service: one requires a field while the other does not, for which we do an in-place update as needed to "correct" the spec. But to support json-merge+patch, we need to support serializing an explicit null to delete data for that field.

Options discussed so far include the following, numbered only for easy reference and not by preference:

  1. Given our agreement for RequestContent<T> and how rarely json-merge+patch support is needed, it may be sufficient to ask callers to pass raw JSON content (as Bytes or str).
  2. For those models used by operations that use content-type: application/json-merge+patch, we could define in azure_core an Option<T>-like tristate enum e.g.,

    pub enum NullableOption<T> {
       Some(T),
       None,
       Null,
    }

    We'd then implement serde::Serialize and serde::Deserialize to handle the different variants much like serde implements support for Option<T>. Null would serialize null much like excluding #[serde(skip_serializing_if = "Option::is_none")] would do for an optional field if None, or elide the field much like including the attribute would if None.

We should look at what other implementations are doing in this scenario.

heaths commented 5 months ago

See https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=ec2c0deb734032010e4c789f5a585166 for an example. Deserialization doesn't quite work: there's no way to know during deserialization whether a null should map to None or Null. That said, mapping both to None by default during deserialization only makes sense since the absence of a field or if the field is set to null should use the default Option<T> value anyway, which would be None.

heaths commented 5 months ago

Looking for other solutions, about the only thing I'm finding is json-patch which has a merge function that takes a couple serde_json::Value parameters, making it dependent on serde_json and not generalized on serde: https://docs.rs/json-patch/1.2.0/src/json_patch/lib.rs.html#642-644

I don't think it's a non-starter, but less than ideal since it relies on compile-time (via the serde_json::json!() macro) or runtime via something like:

let value: Value = r#"{"foo":"bar","baz":1,"qux":null}"#.parse()?;

We really only need something like NullableOption<T> from my playground link for serialization, though it does raise question about server-side generated code down the line and whether it should support deserialization for such instances. /cc @allenjzhang @markcowl