oxidecomputer / typify

compiler from JSON Schema into idiomatic Rust types
Apache License 2.0
393 stars 57 forks source link

Format Email doesn't seem to work in a certain schema #560

Closed Norlock closed 4 months ago

Norlock commented 4 months ago

I have the following schema:

{
  "$schema": "http://json-schema.org/draft-07/schema#",
  "title": "Generated schema for Root",
  "type": "object",
  "properties": {
    "email": {
      "type": "string",
      "anyOf": [
        {
          "format": "email"
        },
        {
          "format": "uri",
          "pattern": "^mailto:[^@]*[^\\.]@[^\\.]($|[^@]*[^\\.]$)"
        }
      ]

    }
  },
  "required": [
    "email"
  ]
}

But it doesn't seem to generate types, I would like to fix this as well. But maybe you can give me some tips on where to begin. I am already working in convert.rs and have made a custom function that will be called if only a format with email is given. (convert_format), I want to create a wrapper around a String, Email(pub String). Is this possible with the current architecture? Thanks in advance

ahl commented 4 months ago

I think (but I'm not sure) that we'd need to add "type": "string" to both of those subschemas for them to be correct (more correct?). That at least avoids typify panicking. The types generated aren't that good because of that anyOf -- still an area of known shortcomings. If you change that to a oneOf (which seems valid) you still don't get such a great type:

#[serde(untagged)]
pub enum GeneratedSchemaForRootEmail {
    Variant0(String),
    Variant1(String),
}

Can I back up and ask what you'd ideally like to see generated? Also, how are you using typify (e.g. builder, macro, cargo-typify)? There are some execution options such as conversion overrides that might be useful to you here.

Norlock commented 4 months ago

Hey

Yes adding that field helps fixing it. For the question on how I would like to see it generated, I was thinking about the following:

format: "email"

It will create a struct like this:

#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
pub struct Email(pub String);

impl Deref for Email {
    type Target = String;
    fn deref(&self) -> &Self::Target {
        &self.0
    }
}

impl DerefMut for Email {
    fn deref_mut(&mut self) -> &mut Self::Target {
        &mut self.0
    }
}

struct EmailVisitor;

impl<'de> Visitor<'de> for EmailVisitor {
    type Value = Email;

    fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
        formatter.write_str("a valid email format")
    }

    fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
    where
        E: serde::de::Error,
    {
        if regress::Regex::new("the-email-regex")
            .unwrap()
            .find(v)
            .is_some()
        {
            Ok(Email(v.to_string()))
        } else {
            Err(E::custom(format!("Email format is not valid: {v}")))
        }
    }
}

impl<'de> Deserialize<'de> for Email {
    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
    where
        D: serde::Deserializer<'de>,
    {
        deserializer.deserialize_str(EmailVisitor)
    }
}

Some values have to be filled correctly in this example like the regex. I didn't know about conversion overrides I can probably use that thanks!

But because these formats are predetermined its better to add them to this repository I guess.

Maybe we can also include the deref + derefmut trait. I don't need to know if its type one or two. As long as it conforms to the format :). So I get an pub struct Email(pub String) or pub struct Uri(pub String).

Which will generate the following enum:


pub enum RootEmailType {
    Email(Email),
    Uri(Uri)
}
Norlock commented 4 months ago

I'm using the builder at the moment, but I will take a look at the macro :+1:

ahl commented 4 months ago

I'm using the builder at the moment, but I will take a look at the macro 👍

Both the builder and macro are capable of conversion overrides. In particular I was thinking that this might be a situation in which you want to provide your own Email type and add a conversion override that directs typify to translate { "type": "string", "format": "email" } to mycrate::Email. You could do something similar for the uri / mailto as well.

That might get us to something like:

pub enum RootEmailType {
    Variant0(Email),
    Variant1(Uri),
}

I have long hoped to get smarted about variant naming, in particular with this case where all variants of an untagged enum come from named types, but it hasn't been a priority for me. But it would be nice, as you say, to land here:

pub enum RootEmailType {
    Email(Email),
    Uri(Uri),
}
Norlock commented 4 months ago

Ok I do think I will do that, we need to make the types once anyway, but thanks for the help.