Properly store text date time values

drift/lib/src/runtime/types/mapping.dart

@@ -11,6 +11,10 @@ import '../query_builder/query_builder.dart';
 /// literals.
 @sealed
 class SqlTypes {
+  // Stolen from DateTime._parseFormat
+  static final RegExp _timeZoneInDateTime =
+      RegExp(r' ?([-+])(\d\d)(?::?(\d\d))?$');
+
   final bool _storeDateTimesAsText;
 
   /// Creates an [SqlTypes] mapper from the provided options.
@@ -25,7 +29,32 @@ class SqlTypes {
     // These need special handling, all other types are a direct mapping
     if (dartValue is DateTime) {
       if (_storeDateTimesAsText) {
+        // sqlite3 assumes UTC by default, so we store the explicit UTC offset
+        // along with the value. For UTC datetimes, there's nothing to change
+        if (dartValue.isUtc) {
           return dartValue.toIso8601String();
+        } else {
+          final offset = dartValue.timeZoneOffset;
+          // Quick sanity check: We can only store the UTC offset as `hh:mm`,
+          // so if the offset has seconds for some reason we should refuse to
+          // store that.
+          if (offset.inSeconds - 60 * offset.inMinutes != 0) {
+            throw ArgumentError.value(dartValue, 'dartValue',
+                'Cannot be mapped to SQL: Invalid UTC offset $offset');
+          }
+
+          final hours = offset.inHours.abs();
+          final minutes = offset.inMinutes.abs() - 60 * hours;
+
+          // For local date times, add the offset as ` +hh:mm` in the end. This
+          // format is understood by `DateTime.parse` and date time functions in
+          // sqlite.
+          final prefix = offset.isNegative ? ' -' : ' +';
+          final formattedOffset = '${hours.toString().padLeft(2, '0')}:'
+              '${minutes.toString().padLeft(2, '0')}';
+
+          return '${dartValue.toIso8601String()}$prefix$formattedOffset';
+        }
       } else {
         return dartValue.millisecondsSinceEpoch ~/ 1000;
       }
@@ -59,7 +88,8 @@ class SqlTypes {
       return dart.toString();
     } else if (dart is DateTime) {
       if (_storeDateTimesAsText) {
-        return "'${dart.toIso8601String()}'";
+        final encoded = mapToSqlVariable(dart).toString();
+        return "'$encoded'";
       } else {
         return (dart.millisecondsSinceEpoch ~/ 1000).toString();
       }
@@ -93,7 +123,19 @@ class SqlTypes {
         return int.parse(sqlValue.toString()) as T;
       case DriftSqlType.dateTime:
         if (_storeDateTimesAsText) {
-          return DateTime.parse(read(DriftSqlType.string, sqlValue)!) as T;
+          final rawValue = read(DriftSqlType.string, sqlValue)!;
+          final value = DateTime.parse(rawValue);
+
+          // The stored format is the same as toIso8601String for utc values,
+          // but for local date times we append the time zone offset.
+          // DateTime.parse picks that up, but then returns an UTC value. For
+          // round-trip equality, we recover that information and reutrn to
+          // a local date time.
+          if (_timeZoneInDateTime.hasMatch(rawValue)) {
+            return value.toLocal() as T;
+          } else {
+            return value as T;
+          }
         } else {
           final unixSeconds = read(DriftSqlType.int, sqlValue)!;
           return DateTime.fromMillisecondsSinceEpoch(unixSeconds * 1000) as T;

drift/test/database/types/datetime_test.dart

@@ -25,4 +25,138 @@ void main() {
 
     expect(ctx.sql, "name < strftime('%s', CURRENT_TIMESTAMP)");
   });
+
+  group('mapping datetime values', () {
+    group('from dart to sql', () {
+      final local = DateTime(2022, 07, 21, 22, 53, 12, 888, 999);
+      final utc = DateTime.utc(2022, 07, 21, 22, 53, 12, 888, 999);
+
+      test('as unix timestamp', () {
+        expect(Variable(local),
+            generates('?', [local.millisecondsSinceEpoch ~/ 1000]));
+        expect(Variable(utc), generates('?', [1658443992]));
+
+        expect(Constant(local),
+            generates('${local.millisecondsSinceEpoch ~/ 1000}'));
+        expect(Constant(utc), generates('1658443992'));
+      });
+
+      test('as text', () {
+        const options = DriftDatabaseOptions(storeDateTimeAsText: true);
+
+        expect(
+          Variable(_MockDateTime(local, const Duration(hours: 1))),
+          generatesWithOptions(
+            '?',
+            variables: ['2022-07-21T22:53:12.888999 +01:00'],
+            options: options,
+          ),
+        );
+        expect(
+          Variable(_MockDateTime(local, const Duration(hours: 1, minutes: 12))),
+          generatesWithOptions(
+            '?',
+            variables: ['2022-07-21T22:53:12.888999 +01:12'],
+            options: options,
+          ),
+        );
+        expect(
+          Variable(
+              _MockDateTime(local, -const Duration(hours: 1, minutes: 29))),
+          generatesWithOptions(
+            '?',
+            variables: ['2022-07-21T22:53:12.888999 -01:29'],
+            options: options,
+          ),
+        );
+
+        expect(
+          Variable(utc),
+          generatesWithOptions(
+            '?',
+            variables: ['2022-07-21T22:53:12.888999Z'],
+            options: options,
+          ),
+        );
+
+        expect(
+          Constant(_MockDateTime(local, const Duration(hours: 1))),
+          generatesWithOptions(
+            "'2022-07-21T22:53:12.888999 +01:00'",
+            options: options,
+          ),
+        );
+        expect(
+          Constant(utc),
+          generatesWithOptions("'2022-07-21T22:53:12.888999Z'",
+              options: options),
+        );
+
+        // Writing date times with an UTC offset that isn't a whole minute
+        // is not supported and should throw.
+        expect(() {
+          final context = stubContext(options: options);
+          Variable(_MockDateTime(local, const Duration(seconds: 30)))
+              .writeInto(context);
+        }, throwsArgumentError);
+
+        expect(() {
+          final context = stubContext(options: options);
+          Constant(_MockDateTime(local, const Duration(seconds: 30)))
+              .writeInto(context);
+        }, throwsArgumentError);
+      });
+    });
+
+    group('from sql to dart', () {
+      test('as unix timestamp', () {
+        const types = SqlTypes(false);
+
+        expect(types.read(DriftSqlType.dateTime, 1658443992),
+            DateTime.utc(2022, 07, 21, 22, 53, 12).toLocal());
+      });
+
+      test('as text', () {
+        const types = SqlTypes(true);
+
+        expect(types.read(DriftSqlType.dateTime, '2022-07-21T22:53:12Z'),
+            DateTime.utc(2022, 07, 21, 22, 53, 12));
+
+        expect(
+          types.read(DriftSqlType.dateTime, '2022-07-21T22:53:12 -03:00'),
+          DateTime.utc(2022, 07, 21, 22, 53, 12)
+              .add(const Duration(hours: 3))
+              .toLocal(),
+        );
+      });
+    });
+  });
+}
+
+class _MockDateTime implements DateTime {
+  final DateTime original;
+  final Duration utcOffset;
+
+  _MockDateTime(this.original, this.utcOffset) : assert(!original.isUtc);
+
+  @override
+  bool get isUtc => false;
+
+  @override
+  Duration get timeZoneOffset => utcOffset;
+
+  @override
+  String toIso8601String() {
+    return original.toIso8601String();
+  }
+
+  @override
+  String toString() {
+    return '${original.toString()} with fake offset $utcOffset';
+  }
+
+  @override
+  dynamic noSuchMethod(Invocation invocation) {
+    return super.noSuchMethod(invocation);
+  }
 }

drift/test/test_utils/matchers.dart

@@ -16,14 +16,24 @@ void expectNotEquals(dynamic a, dynamic expected) {
 /// Matcher for [Component]-subclasses. Expect that a component generates the
 /// matching [sql] and, optionally, the matching [variables].
 Matcher generates(dynamic sql, [dynamic variables = isEmpty]) {
-  return _GeneratesSqlMatcher(wrapMatcher(sql), wrapMatcher(variables));
+  return _GeneratesSqlMatcher(
+      wrapMatcher(sql), wrapMatcher(variables), const DriftDatabaseOptions());
+}
+
+Matcher generatesWithOptions(dynamic sql,
+    {dynamic variables = isEmpty,
+    DriftDatabaseOptions options = const DriftDatabaseOptions()}) {
+  return _GeneratesSqlMatcher(
+      wrapMatcher(sql), wrapMatcher(variables), options);
 }
 
 class _GeneratesSqlMatcher extends Matcher {
   final Matcher _matchSql;
   final Matcher? _matchVariables;
 
-  _GeneratesSqlMatcher(this._matchSql, this._matchVariables);
+  final DriftDatabaseOptions options;
+
+  _GeneratesSqlMatcher(this._matchSql, this._matchVariables, this.options);
 
   @override
   Description describe(Description description) {
@@ -68,7 +78,7 @@ class _GeneratesSqlMatcher extends Matcher {
       return false;
     }
 
-    final ctx = stubContext();
+    final ctx = stubContext(options: options);
     item.writeInto(ctx);
 
     var matches = true;