Include timezone when serializing JSON responses

[jtgeibel]

This adds back the timezone information to JSON responses. Note that the format is slightly different than we provided from the old time crate. Previously, responses would end in Z and now the timezone is provided as +00:00. However, both representations are compliant with RFC3339.

Because of this regression, cargo-info has applied a patch to handle responses that may or may not include the timezone information. I've locally tested cargo-info both with and without this patch to confirm that both versions correctly handle the format with +00:00.

The code for handling Option is a bit more complicated that I would like (given that we only have one instance of this), but I couldn't find a way to direct serde to handle this automatically.

Fixes #1123

[imp]

Out of curiosity - why you opted to keep the timestamps in NaiveDateTime and convert them while serializing, instead of just changing it to DateTime<Utc> and have the desired format automagically ?

[jtgeibel]

@imp I originally tried changing things to DateTime<Utc> but ran into some errors I couldn't resolve. @sgrif is there a better way I could do this directly in diesel? All our timestamp columns are "without time zone" so I'm assuming not. I also don't think it would make sense to change the columns either since we don't need to deal with multiple time zones and UTC is always implied.

[imp]

@jtgeibel Admittedly I don't know much about interaction with diesel, but naive (pun unintended) approach would be stick to NaiveDateTime in native data structures (if mandated by diesel) while changing it to DateTime<Utc> in respective Encodable.... And then translate when creating the latter from the former.
For example:

struct Version {
    updated_at: NaiveDateTime,
   ...
}

struct EncodableVersion {
    updated_at: DateTime<Utc>,
    ...
}

impl Version {
    pub fn encodable(self, crate_name: &str) -> EncodableVersion {
        EncodableVersion {
            updated_at:  DateTime::<Utc>::from_utc(self.updated_at, Utc);
            ...
        }
    }
}

From this point on serde will do the right thing.
However, that is my uneducated guess - I definitely do not see the big picture.

[jtgeibel]

@imp Yeah that is the other option that I considered and is similar to what the previous code (with the time crate) did. I think the serde attributes are a bit cleaner, with the exception of that pesky Option<>.

[imp]

@jtgeibel Very well, thanks for explaining!

[carols10cents]

This looks great! The one request I have is could we add a test for at least one of these, setting a time to a known value and checking the JSON datetime against a static string, to hopefully prevent us from inadvertently breaking this in the future? :)

[carols10cents]

Adding tests for this might also be a good task for someone new to take on, so I'm also okay with merging this as-is and opening an issue for tests. What do you think?

[carols10cents]

Let's merge this and make a new issue for the tests :)

bors: r+

[bors-voyager]

Build succeeded

• continuous-integration/travis-ci/push