Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Custom (de)serializers #782

Open
djrodgerspryor opened this issue Sep 27, 2023 · 2 comments · May be fixed by #980
Open

Custom (de)serializers #782

djrodgerspryor opened this issue Sep 27, 2023 · 2 comments · May be fixed by #980
Labels
breaking A breaking change
Milestone
4.0.0

Comments

@djrodgerspryor
Copy link

Thanks for making cynic: it's great!

I'm currently using it to talk to an API which works with non-rfc3339 timestamps (they look like: 2023-09-27T18:42:31 - note the lack of an offset at the end). I've added chrono::DateTime as a scalar type as per the docs:

cynic::impl_scalar!(chrono::DateTime<chrono::Utc>, schema::ISODateTime);

But the lack of an offset makes chrono grumpy when parsing these values. Normally I'd handle this with a custom deserialize_with helper in serde. Something like:

fn generous_datetime_parser<'de, D>(
    deserializer: D,
) -> Result<chrono::DateTime<chrono::Utc>, D::Error>
where
    D: serde::Deserializer<'de>,
{
    use serde::Deserialize;

    let value = String::deserialize(deserializer)?;

    chrono::DateTime::parse_from_rfc3339(value.as_str())
        // Try adding a timezone (assume UTC) since those are often missing
        .or_else(|_| chrono::DateTime::parse_from_rfc3339(format!("{}+00:00", &value).as_str()))
        // Try adding a time component in case we were just given a date
        .or_else(|_| {
            chrono::DateTime::parse_from_rfc3339(format!("{}T00:00:00+00:00", &value).as_str())
        })
        .map_err(|e| {
            serde::de::Error::custom(format!(
                "Unable to parse chrono::DateTime from string ({}): {:?}",
                value, e
            ))
        })
        .map(|dt| dt.with_timezone(&chrono::Utc))
}

...

#[derive(cynic::QueryFragment, Debug, serde::Serialize)]
#[cynic(schema = "my_schema")]
struct Foo {
    #[serde(deserialize_with = "generous_datetime_parser")]
    created: chrono::DateTime<chrono::Utc>,
}

Using the deserialize_with serde option as above actually compiles, but it seems to do nothing, I'm guessing because of how cynic handles deserializing the scalar types internally. Is there already a supported way to do this in cynic, or at least a workaround to hook into the deserialization logic?
@obmarg
Copy link
Owner

obmarg commented Sep 27, 2023

Hey @djrodgerspryor - thanks for the report.

The best way to handle this at the moment is probably to use a custom newtype for those fields:

pub struct ISODateTime(chrono::DateTIme<chrono::Utc>);

impl Serialize for ISODateTime {
   // Put your custom serialize code here
}

impl Deserialize<'static> for ISODateTime {
   // Put your custom serialize code here
}

cynic::impl_scalar!(ISODateTime, schema::ISODateTime);

Does that work for you or is it a poor workaround?

I'm not against looking into alternatives if not. Which might be similar to deserialize_with from serde, though I would really like a way to tie it to the type so you don't have to repeat yourself everywhere you use one of these fields.

@djrodgerspryor
Copy link
Author

Thanks! The newtype approach is a decent workaround (and let me do some other things like converting GQL strings into structured rust data). I do think it would makes sense for cynic to move away from requiring newtypes though in the spirit of using your own — fully controlled — struct and not a GQL-specific struct like other crates require.

I haven't dug into the internals to check the feasibility of this, but would it be possible to expose a BYO-(De)Serialize variant of cynic::QueryFragment? That way, the user could opt into using serde directly, and automatically get all of it's features (deserialize_with, rename_all etc.) or just implement totally custom serialization logic if needed.

@obmarg obmarg added this to the 4.0.0 milestone Apr 7, 2024
@obmarg obmarg added the breaking A breaking change label Jun 8, 2024
@obmarg obmarg linked a pull request Jun 9, 2024 that will close this issue
Draft
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
breaking A breaking change
Projects
None yet
Milestone
4.0.0
Development

Successfully merging a pull request may close this issue.

feat: custom ser/de for scalars obmarg/cynic
2 participants
@obmarg @djrodgerspryor