Skip to content

Issues with NaiveDateTime and Query Variables #775

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.

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

Closed
Nocturem opened this issue Oct 6, 2020 · 2 comments · Fixed by #777
Closed

Issues with NaiveDateTime and Query Variables #775

Nocturem opened this issue Oct 6, 2020 · 2 comments · Fixed by #777
Assignees
Labels
bug Something isn't working

Comments

@Nocturem
Copy link

Nocturem commented Oct 6, 2020

Hi there,

I have run into a little trouble making an Apollo frontend play nice with my Juniper + Diesel + Rocket backend specifically in handling NaiveDateTime values within an input object

I have a GraphQL input object

use chrono::NaiveDateTime;

#[derive(juniper::GraphQLInputObject, Insertable, AsChangeset)]
#[table_name = "certificates"]
pub struct NewCertificate {
    pub name: String,
    pub version: i32,
    pub serial_no: String,
    pub pub_key: String,
    pub valid_from: NaiveDateTime,
    pub valid_till: NaiveDateTime,
    pub service_id: i32,
}

this is called in

    fn create_certificate(context: &Context, data: NewCertificate) -> FieldResult<Certificate> {
        Ok(
            diesel::insert_into(certificates::table)
            .values(&data)
            .get_result(&context.db.get().unwrap())?
        )
    }

which behaves fine from graphiql as an inline declaration

mutation{
  createCertificate(
    data: {
      name:"cert.domain.net",
      version:3,
      serialNo:"2F:F2:B3:F1:9B:9A:9A:07:B9:F2:C0:69:12:62:C8:F8",
      pubKey:"EE:11:57:13:16:4F:C3:1C:7C:E1:15:8D:C7:F9:4B:01:EA:47:D1:46:93:78:C7:08:B3:BD:7F:D2:91:64:BE:C9",
      validFrom:1601942400,
      validTill:1696550400,
      serviceId:12
    }
  )
  {
    id
    name
  }
}

but doesn't play so nice when called as a variable in a query

mutation($certificate: NewCertificate!){
  createCertificate(data: $certificate){
    id
    name
    version
    serialNo
    pubKey
    validFrom
    validTill
  }
}

with the query variables

{
  "certificate": {
    "name": "cert.domain.net",
    "version": 3,
    "serialNo": "2F:F2:B3:F1:9B:9A:9A:07:B9:F2:C0:69:12:62:C8:F8",
    "pubKey": "EE:11:57:13:16:4F:C3:1C:7C:E1:15:8D:C7:F9:4B:01:EA:47:D1:46:93:78:C7:08:B3:BD:7F:D2:91:64:BE:C9",
    "validFrom": 1601942400,
    "validTill": 1696550400,
    "serviceId":12
    }
}

there we run into the rejection

{
  "errors": [
    {
      "message": "Variable \"$certificate\" got invalid value. In field \"validFrom\": Expected \"NaiveDateTime\".",
      "locations": [
        {
          "line": 1,
          "column": 10
        }
      ]
    },
    {
      "message": "Variable \"$certificate\" got invalid value. In field \"validTill\": Expected \"NaiveDateTime\".",
      "locations": [
        {
          "line": 1,
          "column": 10
        }
      ]
    }
  ]
}

not sure if this is a mistake of wrong import or an issue somewhere else but I would have imagined it should be handling variables the same as inline declarations.

Cheers
Pearce

@LegNeato LegNeato added the bug Something isn't working label Oct 7, 2020
@LegNeato
Copy link
Member

LegNeato commented Oct 7, 2020

This looks like a bug at first glance, thanks for the report!

I hope to get some time to look into this but if you want to start poking around the deserialization is defined in https://github.com/graphql-rust/juniper/blob/master/juniper/src/integrations/chrono.rs

@tyranron
Copy link
Member

tyranron commented Oct 7, 2020

Yup, the problem is in this line, which is not covered by tests, as tests use InputValue directly.

If we look at f64::from_str we will see, that it's not designed to parse usual int numbers, but we do so.

I guess, we should do something like f64::from_str(val).or_else(|| u64::from_str(val).and_then(f64::try_from))

@tyranron tyranron self-assigned this Oct 7, 2020
@tyranron tyranron added k::api Related to API (application interface) and removed k::api Related to API (application interface) labels Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants