Fix: change caa_identities field to be optional in DirectoryMetadata

[mdegel]

Fix for #73

[github-actions]

✅ All required contributors have signed the F5 CLA for this PR. Thank you!
_{Posted by the CLA Assistant Lite bot.}

[mdegel]

I have hereby read the F5 CLA and agree to its terms

[bavshin-f5]

Huh, I expected #[serde(default)] to handle that. Once again I forgot about this particular quirk of serde.

We don't care if this field is missing, null or an empty array and don't want to represent these cases differently. Therefore, instead of wrapping the value in Option, I would prefer to deserialize null as Default. Something like that:

    #[serde(deserialize_with = "deserialize_null_as_default")]
    pub caa_identities: Vec<String>,

...

fn deserialize_null_as_default<'de, D, T>(deserializer: D) -> Result<T, D::Error>
where
    D: serde::de::Deserializer<'de>,
    T: serde::de::Deserialize<'de> + Default,
{
    Option::<T>::deserialize(deserializer).map(Option::unwrap_or_default)
}

[mdegel]

I assume you're talking about serde-rs/serde#1098?

I see your point. If you think this would be the best solution, I can adjust the PR to implement it like this.

However, doesn't it lead to more confusion in the code if all other places use Option, except this one which uses a special function?

[bavshin-f5]

I assume you're talking about serde-rs/serde#1098?

And serde-rs/json#376, where Option<T> does not distinguish between null and a missing value. And a workaround in serde_with::rust::double_option...

I see your point. If you think this would be the best solution, I can adjust the PR to implement it like this.

However, doesn't it lead to more confusion in the code if all other places use Option, except this one which uses a special function?

Not really. As I already mentioned, here we would want to distinguish between a presence of values ("caaIdentities": ["example.com"]) and absence ([], null or omitted field). Option is necessary for non-collection types, is_empty() is good enough for collections. And double wrapping only makes the checks more cumbersome.
Note that we are limited to the Rust toolchain available in popular operating systems, and won't be able to leverage feature(let_chains) at least until the next release of Debian.

However, all of that is not important. Apparently, the only specification that has use for CAA identities is draft-sheurich-acme-dns-persist and I have no plans to support that one yet. So the field is unused, and it is fine to merge it as is.

Thanks for the patch!