Serialization of Local DateTime

[mfulgo]

Serialization/deserialization for ISO-8601 DateTime was added in #111.

When first using the library, I was a little surprised to find out that a var created from new DateTime.now() didn't serialize. While I agree serializing something that isn't UTC doesn't make sense (in most cases), why not call .toUtc() in the serializer?

Advantages:

• Less thought needed for the API user. (I don't need to remember to call .toUtc() every time I create a DateTime.)
• Avoids a runtime exception. (It's nice when APIs catch incorrect usage at compile time.)

Disadvantages:

• It might be considered a breaking change.
• Some API users may be caught off guard, expecting a local DateTime to exclude the timezone or to include their local timezone.

Perhaps making the serializer configurable could accomplish the best balance:

final ser = new Iso8601DateTimeSerializer(); // throws exception on non-UTC
// These would return a new instance
ser.withLocalDateTimeInUtc();                // uses .toUtc().toIso8601String()
ser.withNoZoneForLocalDateTime();            // uses .toIso8601String()

[davidmorgan]

Thanks.

Calling toUtc would make the deserialized result different to what you serialized; this would break a pretty fundamental guarantee about serialization.

I can't think of any way of dealing with local time DateTimes that works well with serialization, so while it's not ideal to throw, I think this is the best option. Local times shouldn't be allowed anywhere near your backend code :)

[mfulgo]

I don't think it really breaks that guarantee[1]. Ultimately, the value that you're serializing is the same instant regardless of which timezone it was originally created in. This is the case when performing deserialization:

final nonUtc = '1980-01-02T05:34:05.006007+02:30';
final deserialized = serializers.deserialize(nonUtc, specifiedType: FullType(DateTime));
expect(deserialized, new DateTime.utc(1980, 1, 2, 3, 4, 5, 6, 7));
final serialized = serializers.serialize(deserialized, specifiedType: FullType(DateTime));
expect(serialized, '1980-01-02T03:04:05.006007Z');

---

1: Unless we're talking about the last case, excluding a timezone, about which I don't really care. If someone really wants to support that case, they can extend the serializer or create their own. At that point, they are either being very intentional or have no clue about what they're doing.

[davidmorgan]

Hmm, what breaks is that you give it a DateTime in local timezone and you get it back after serialization in UTC. Which will cause it to display differently, compare differently, etc. Lots of confusion...

[mfulgo]

That is exactly what happens with the current (de)serializer though: You give it a serialized version with a local timezone (offset) and when you re-serialize it, it is in UTC. (The expectations in the snippet above pass.) The same thing happens if you omit the offset - it changes from an un-zoned DateTime to one with a local offset, which is probably more incorrect: If you throw an exception when serializing local DateTimes, you should probably do the same when deserializing them.

The crux of the issue may be that a "local" DateTime in Dart is not the same as a LocalDateTime in Java 8+ (vs. a ZonedDateTime). In Dart, the object always has a timezone offset, sometimes, it's just not 0.

Ultimately, I think there's some improvement that can be made to the API by either just calling .toUtc() in the serializer or by doing something slightly more involved by adding the timeZoneOffset when serializing local ones. I'll probably create a custom serializer to do this in my own code.

I think this discussion is worth having and having documented. It's ultimately your call as to what the provided one does.

[davidmorgan]

Very good point. I had only considered serialize->deserialize. I keep forgetting people get these values from other places :/

It would be nice to throw on deserialize as well--in any case where it's not output we would have produced by serializing. But I realize this is not necessarily something people can control. So we probably do need something.

Is there any kind of sensible standard for what existing APIs do with offsets?

Thanks!

[mfulgo]

Heh... "sensible standard"? I wish. 😝

Some parallels here are Java and JodaTime. In both cases, they distinguish between a class with zone/offset information (ZonedDateTime/DateTime) and one without a zone/offset (LocalDateTime). Since Dart only has the one DateTime class getting used for both use-cases, I think the confusion stems from there.

That said, maybe the best parallel here is Java's Instant which simply requires a value in UTC.

[dave26199]

Thanks. I'll be in vacation for a week--will get back to this when I'm back!

[zoechi]

What about a way to specify on a field if the serializer should call toUtc()/toLocal() automatically on serialization/deserialization or throw or at least a way to register different de/serializers for DateTime to specify the behavior for all DateTime fields at once?

[kentcb]

I have a back-end that requires non-UTC because it want's to know the local time that data was captured. That time will never be sent back to the client. Anyway, I agree it would be better stored as UTC, possibly with timezone information, but that's not what I've got.

For now, I will need to write my own serializer to use instead of the standard Iso8601DateTimeSerializer. However, I wanted to suggest maybe just taking a flag in the ctor. Something like:

class Iso8601DateTimeSerializer implements PrimitiveSerializer<DateTime> {
  Iso8601DateTimeSerializer({
    this.allowLocal = false,
  });

  final bool allowLocal;

  @override
  final Iterable<Type> types = const [DateTime];

  @override
  final String wireName = 'DateTime';

  @override
  Object serialize(Serializers serializers, DateTime dateTime, {FullType specifiedType = FullType.unspecified}) {
    if (!allowLocal && !dateTime.isUtc) {
      throw ArgumentError.value(dateTime, 'dateTime', 'Must be in UTC for serialization, or the allowLocal flag must be set to true.');
    }

    return dateTime.toIso8601String();
  }

  @override
  DateTime deserialize(Serializers serializers, Object serialized, {FullType specifiedType = FullType.unspecified}) {
    final result = DateTime.parse(serialized as String);

    if (!allowLocal && !result.isUtc) {
      // throw some kind of error here
    }

    return result;
  }
}

[davidmorgan]

That sounds sensible. Not sure when I'll get to it, though :)

[dhrn]

any update on this guys?