Hive: DateTimeAdapter don't take timezone in consideration

[JCKodel]

Steps to Reproduce
DateTime fields are saved used local DateTime.
My data is synced to a server which will restore the data on a phone change/reinstall.
If the user is in a different timezone, all times are wrong when restoring the database.

The DateTimeAdapter should be writer.writeInt(obj.toUtc).millisecondsSinceEpoch) and DateTime.fromMillisecondsSinceEpoch(micros, isUtc: true).toLocal().

Version

• Platform: iOS, Android, Mac, Windows, Linux, Web
• Flutter version: [e.g. 1.5.4]
• Hive version: [e.g. 0.5.0]

[TheMisir]

Isn't DateTime.millisecondsSinceEpoch time zone independent (uses UTC time)? I think we just need to add isUtc: true to read method.

[zs-dima]

@leisim @TheMisir
for example we using gRPC and bloc to process data classes
when we receive dates from the gRPC - isUtc: true
when we save restore object with date fields - isUtc: false

then when we compare object - objects are different that lead to additional redraw UI in case if bloc had been used
or any other possible issues

[leisim]

@zs-dima Thanks for pinging me, this is actually a problem since we cannot change the behaviour without breaking all of the existing apps. I propose you register a custom DateTime adapter and we update the docs to notify users of this quirk.

[zs-dima]

@leisim thanks for the fast reply

I propose you register a custom DateTime adapter

I tried it but Hive use internal adapter instead:
https://github.com/hivedb/hive/blob/master/hive/lib/src/adapters/date_time_adapter.dart

as temporary workaround I converting DateTime to/from UTC manually but could be nice to fix it as it is not obvious behavior

[leisim]

@zs-dima You are right, I just checked the TypeRegistry implementation and it will use the first matching adapter (depending on the time of registry). Since the internal adapters are always registered first, they will be prefered.

@TheMisir I propose to change the logic to use the newest matching adapter. I believe this would be a non breaking change. The only important thing is that the reversing logic is done at registry time and not at lookup time to improve performance. What do you think?

[zs-dima]

hm, performance important
maybe add possibility to register internal adapters manually instead?

[leisim]

hm, performance important

Since the reversing of the adapter registry will be performed at registry time, it will only have a small O(1) performance penalty when you register the adapter. Nothing to worry about.

maybe add possibility to register internal adapters manually instead?

This would be a breaking change.

[zs-dima]

@leisim @TheMisir
Idea - what about add isUtc check for the current adapter and keep behavior for the isUtc==false and uppdate just for the isUtc==true ?
So should not affect current users.
And it is more like adapter bug fix then changes.

[leisim]

what about add isUtc check for the current adapter and keep behavior for the isUtc==false and uppdate just for the isUtc==true ?

That's a genius idea :+1:

[zs-dima]

looks obvious to get 'isUtc: false' date from DB in case when we saved 'isUtc: false'
and get 'isUtc: true' in case when we saved 'isUtc: true'
so looks isUtc value have to be saved as well

[TheMisir]

@TheMisir I propose to change the logic to use the newest matching adapter. I believe this would be a non breaking change. The only important thing is that the reversing logic is done at registry time and not at lookup time to improve performance. What do you think?

But, TypeRegistry saves adapters inside Map<int, TypeAdapter>. So, I think DateTime adapter could be overwritten by using same typeId as DateTimeAdapter.

Here's sample DateTime adapter which forces datetime to use UTC timezone:

import 'package:hive/hive.dart';

/// Adapter for DateTime
class UtcDateTimeAdapter extends TypeAdapter<DateTime> {
  @override
  final typeId = 16;

  @override
  DateTime read(BinaryReader reader) {
    var micros = reader.readInt();
    return DateTime.fromMillisecondsSinceEpoch(micros, isUtc: true);
  }

  @override
  void write(BinaryWriter writer, DateTime obj) {
    writer.writeInt(obj.toUtc().millisecondsSinceEpoch);
  }
}

I think we might need to expose isInternal argument to public in Hive.registerAdapter.

[zs-dima]

@TheMisir, thanks for reply

1. I tried to override adapter in this way but it doesn't work for some reason
2. Adapter have to save isUtc bool value and restore on load from Hive. Dart account isUtc true and false dates as different objects.

[TheMisir]

1. I tried to override adapter in this way but it doesn't work for some reason

Yes, that's why I said "we might need to expose isInternal argument" which is used to separate internal adapters from external ones (DateTimeAdapter is currently internal adapter).

1. Adapter have to save isUtc bool value and restore on load from Hive. Dart account isUtc true and false dates as different objects.

After adding custom internal adapters support It will be possible to write time zone info or isUtc value.

Btw, I suggest storing and transmitting (to/from server) date in UTC otherwise you'll see lots of weird behavior between multiple timezones especially If you need to store date on server side. This behavior took my months to make up everything is working as expected.

[TheMisir]

@leisim ,

Should I add isInternal argument to registerAdapter method which is currently only available inside hive itself (TypeRegistry doesn't contains isInternal argument but TypeRegistryImpl does which is not available on public API)?

[zs-dima]

@TheMisir
It could be nice to be able to override internal adapters, yes.
However for the default datetime adapter could be nice to save isUtc, so server dta cache and etc will save-restore isUtc:true, however for the settings and some in-app objects users could use isUtc: false and expect to restore datetime with same format as it had been saved.

[zs-dima]

It took time for me to find out app issue related to this datetime equally, so I hope default datetime adapter will be fixed for all users.

[TheMisir]

@TheMisir
It could be nice to be able to override internal adapters, yes.
However for the default datetime adapter could be nice to save isUtc, so server dta cache and etc will save-restore isUtc:true, however for the settings and some in-app objects users could use isUtc: false and expect to restore datetime with same format as it had been saved.

Changing data structure (saving isUtc) will be breaking change. Instead we'll probably provide an option for overriding internal type adapters.

[leisim]

Instead we'll probably provide an option for overriding internal type adapters.

Yes, I believe this is the way to go. Maybe we can even provide a DateTimeWithTimezoneAdapter which can be used to manually override the internal adapter

[TheMisir]

I write backward compatible DateTime adapter which will save timezone info for new writes also able to read datetime values with or without timezone info.

class DateTimeWithTimezoneAdapter extends TypeAdapter<DateTime> {
  @override
  final typeId = 16;

  @override
  DateTime read(BinaryReader reader) {
    var micros = reader.readInt();
    var isUtc = reader.availableBytes > 0 ? reader.readBool() : false;
    return DateTime.fromMillisecondsSinceEpoch(micros, isUtc: isUtc);
  }

  @override
  void write(BinaryWriter writer, DateTime obj) {
    writer.writeInt(obj.millisecondsSinceEpoch);
    writer.writeBool(obj.isUtc);
  }
}

[leisim]

var isUtc = reader.availableBytes > 0 ? reader.readBool() : false;

Unfortunately, this will only work for DateTimes which are stored on their own. If this is used to store a DateTime field of another object, reader.availableBytes will return the remaining bytes of the object and not of the DateTime.

What we can do is register the DateTimeWithTimezoneAdapter with a new typeId (18?) and make sure Hive prefers it to the old DateTimeAdapter. This way Hive will use the old adapter for deserializing existing DateTimes and the new adapter for serializing and deserializing newly stored DateTimes.

[TheMisir]

var isUtc = reader.availableBytes > 0 ? reader.readBool() : false;

Unfortunately, this will only work for DateTimes which are stored on their own. If this is used to store a DateTime field of another object, reader.availableBytes will return the remaining bytes of the object and not of the DateTime.

What we can do is register the DateTimeWithTimezoneAdapter with a new typeId (18?) and make sure Hive prefers it to the old DateTimeAdapter. This way Hive will use the old adapter for deserializing existing DateTimes and the new adapter for serializing and deserializing newly stored DateTimes.

What do you think about this implementation ?

[zs-dima]

@leisim @TheMisir
Any updates are more than welcome, hope you will have time to provide some solution or workaround

[TheMisir]

@leisim @TheMisir
Any updates are more than welcome, hope you will have time to provide some solution or workaround

I'm working on it.

[TheMisir]

I have just published [email protected]. Now It does stores datetimes with timezone info.

To get latest version you have to update pubspec.yaml file.

dependencies:
  hive: ^1.5.0-pre

[zs-dima]

@TheMisir @leisim
Thanks a lot, well done

[peterwvj]

Thanks, @TheMisir . When do you expect to promote the 1.5.0-pre release?

[TheMisir]

Thanks, @TheMisir . When do you expect to promote 1.5.0-pre release?

I think you can set version in pubspec.yaml as ^1.5.0-pre which will upgrade to next stable versions when available. I'll promote 1.5.0-pre to stable in coming week probably. I'm not sure exactly because my workload increased a bit during this year, so I could not spend much time on hive.

[kinex]

I don't think there is this issue. The problem is probably that you are comparing DateTimes using the == operator when you should actually use the method DateTime.isAtSameMomentAs. == operator returns true only if the DateTimes are on the same time zone which is usually not important.

DateTime.microsecondsSinceEpoch is independent from the time zone. Similarly DateTime.fromMillisecondsSinceEpoch returns always the same date/time regardless of the isUtc parameter. isUtc parameter only controls whether the DateTime object is initialized in local or in UTC time. It is still the exactly same date/time when compared correctly using the method DateTime.isAtSameMomentAs.

You can play with this unit test to verify these:

    test('DateTime', () {
      final now = DateTime.now();

      expect(now.microsecondsSinceEpoch == now.toUtc().microsecondsSinceEpoch,
          isTrue);

      final utcNow = DateTime.now().toUtc();
      expect(
          utcNow.microsecondsSinceEpoch ==
              utcNow.toLocal().microsecondsSinceEpoch,
          isTrue);

      expect(
          DateTime.fromMicrosecondsSinceEpoch(1000, isUtc: false)
              .isAtSameMomentAs(
                  DateTime.fromMicrosecondsSinceEpoch(1000, isUtc: true)),
          isTrue);
    });