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.

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

Support for parsing/formatting as RFC-3339 with Serde #387

Closed
sazzer opened this issue Nov 5, 2021 · 7 comments
Closed

Support for parsing/formatting as RFC-3339 with Serde #387

sazzer opened this issue Nov 5, 2021 · 7 comments
Labels
A-third-party Area: implementations of traits from other crates C-feature-request Category: a new feature (not already implemented)

Comments

@sazzer
Copy link

sazzer commented Nov 5, 2021

Issue #232 has added support for parsing and formatting timestamps using the RFC-3339. However, as best as I can tell, it only supports this in explicit code. It would be useful if this was supported in the Serde feature as well.

For example, instead of needing to do:

#[derive(Serialize)]
struct SomeResponse {
    pub some_timestamp: String
}
.....
SomeResponse {
    some_timestamp: value.format(time::Format::Rfc3339).expect("Failed to format value")
}

I could just do:

#[derive(Serialize)]
struct SomeResponse {
    #[serde(with = "time::serde::rfc3339")]
    pub some_timestamp: OffsetDateTime
}
.....
SomeResponse {
    some_timestamp: value
}
@jhpratt
Copy link
Member

jhpratt commented Nov 5, 2021

Yeah, this is doable.

@jhpratt jhpratt added A-third-party Area: implementations of traits from other crates C-feature-request Category: a new feature (not already implemented) labels Nov 5, 2021
@jhpratt
Copy link
Member

jhpratt commented Nov 11, 2021

This should be completed (sans tests) on the serde-rfc3339 branch. I will not be releasing this until weak feature dependencies has been on stable for the requisite amount of time, as doing so would mean anyone using a fair number of APIs would pull in serde for no apparent reason.

@jhpratt jhpratt added the C-blocked Category: blocked by another (possibly upstream) issue label Nov 11, 2021
clintfred added a commit to IronCoreLabs/ironoxide that referenced this issue Nov 19, 2021
…`time::OffsetDateTime`.

This change should not have any semantic meaning and is being done to address RUSTSEC-2020-0071 (see #244) and RUSTSEC-20200159 (see #245).

This is a breaking API change because some result types include create/updated timestamps. Consumers of the library will need to add a direct dependency on `time`. Note that `chrono` already uses `time` internally, so this change should not bloat transitive dependencies.

For those needing serialization support of  `OffsetDateTime` timestamps, the `time` crate offers this support with the `serde` feature flag.

One small point of friction here is rfc3339 support. `time` has this support on [a branch](time-rs/time#387) that is blocked on a rust feature hitting stable, so I copied in a few lines of code from that branch to support deserializing rfc3339 timestamps.
clintfred added a commit to IronCoreLabs/ironoxide that referenced this issue Nov 23, 2021
…`time::OffsetDateTime` (#249)

* Removes `chrono` and  usages  of `DateTime<Utc>` and replaces them with`time::OffsetDateTime`.

This change should not have any semantic meaning and is being done to address RUSTSEC-2020-0071 (see #244) and RUSTSEC-20200159 (see #245).

This is a breaking API change because some result types include create/updated timestamps. Consumers of the library will need to add a direct dependency on `time`. Note that `chrono` already uses `time` internally, so this change should not bloat transitive dependencies.

For those needing serialization support of  `OffsetDateTime` timestamps, the `time` crate offers this support with the `serde` feature flag.

One small point of friction here is rfc3339 support. `time` has this support on [a branch](time-rs/time#387) that is blocked on a rust feature hitting stable, so I copied in a few lines of code from that branch to support deserializing rfc3339 timestamps.

* depend on our fork of jsonwebtoken

* cargo fmt

* lock to specific rev on jsonwebtoken fork

* self review changes

* Update to new jsonwebtoken

Co-authored-by: Colt Frederickson <coltfred@gmail.com>
@mzabaluev
Copy link
Contributor

as doing so would mean anyone using a fair number of APIs would pull in serde for no apparent reason.

I'm not sure I understand the concern here. The whole mod serde is gated by the serde feature, use of time::serde::rfc3339 further needs parsing and formatting, hence std (note this could be eased to alloc by #400). But anyone using this helper module must be needing serde anyway, aren't they?

@jhpratt
Copy link
Member

jhpratt commented Dec 3, 2021

I have a plan to get this to work. I am just working on something else first.

@jhpratt
Copy link
Member

jhpratt commented Dec 17, 2021

I thought about putting this behind the serde-human-readable flag, but that would require the default serialization to be human readable just to support the attributes. As a result, I decided to put this behind a new serde-well-known feature flag.

This is on main now — both RFC3339 and RFC2822. Tests will be added for this soon.

@jhpratt jhpratt closed this as completed Dec 17, 2021
@jhpratt jhpratt removed the C-blocked Category: blocked by another (possibly upstream) issue label Dec 17, 2021
@George-Miao
Copy link

Hi! Are serde-well-known available on crates.io now?

@jhpratt
Copy link
Member

jhpratt commented Dec 20, 2021

Not yet, no. It was just merged recently. There be a release soon-ish once some other changes get finalized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-third-party Area: implementations of traits from other crates C-feature-request Category: a new feature (not already implemented)
Projects
None yet
Development

No branches or pull requests

4 participants