firebase / flutterfire

🔥 A collection of Firebase plugins for Flutter apps.
https://firebase.google.com/docs/flutter/setup
BSD 3-Clause "New" or "Revised" License
8.72k stars 3.97k forks source link

🐛 Cloud Firestore - microseconds are lost when implicitly converting DateTime to Timestamp #12102

Open kwill39 opened 10 months ago

kwill39 commented 10 months ago

Bug report

Describe the bug The microseconds of DateTime are lost when working with ToFirestore<R> for a withConverter for a CollectionReference being implicitly converted to Timestamp.

Steps to reproduce

Steps to reproduce the behavior:

  1. Tap the floating action button
  2. Observe terminal output with expected result and actual result
  3. Repeat as desired with different DateTime inputs

Expected behavior

DateTime should be converted without the loss of microseconds.

Sample project

This bug requires a Firebase project to reproduce. As such, the code needed to reproduce the problem is being provided in place of a repository URL. Anyone who would like to reproduce the bug is encouraged to create a new Firebase project in order to compile the following code sample. The code sample provided makes use of the Firestore emulator.

[!NOTE] The following code sample uses a custom object with ToFirestore and FromFirestore. There is a more concise version of the code sample, in a separate comment below, that reproduces the bug and does not involve a custom object.

import 'package:cloud_firestore/cloud_firestore.dart';
import 'package:firebase_core/firebase_core.dart';
import 'package:flutter/material.dart';

import 'firebase_options.dart';

void main() async {
  WidgetsFlutterBinding.ensureInitialized();
  await Firebase.initializeApp(
    options: DefaultFirebaseOptions.currentPlatform,
  );
  FirebaseFirestore firestore = FirebaseFirestore.instance;
  firestore.useFirestoreEmulator('localhost', 8080);
  Settings settings = const Settings(persistenceEnabled: false);
  firestore.settings = settings;
  runApp(const MyApp());
}

class CustomDateTime {
  CustomDateTime(this.dateTime);

  final DateTime dateTime;

  static CustomDateTime fromFirestore(
    DocumentSnapshot<Map<String, dynamic>> snapshot,
    SnapshotOptions? options,
  ) {
    Timestamp timestamp = snapshot.data()!['date'] as Timestamp;
    DateTime date = timestamp.toDate().toLocal();
    return CustomDateTime(date);
  }

  static Map<String, Object?> toFirestore(
    CustomDateTime customDateTime,
    SetOptions? options,
  ) {
    return <String, Object?>{
      'date': customDateTime.dateTime,
    };
  }
}

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: Scaffold(
        floatingActionButton: FloatingActionButton(
          onPressed: () async {
            int oneMicrosecond = 1;
            DateTime dateTime = DateTime(2023, 11, 1, 0, 0, 0, 0, oneMicrosecond);
            CustomDateTime customDateTime = CustomDateTime(dateTime);
            CollectionReference<CustomDateTime> collectionRef =
                FirebaseFirestore.instance
                    .collection('dates')
                    .withConverter<CustomDateTime>(
                      fromFirestore: CustomDateTime.fromFirestore,
                      toFirestore: CustomDateTime.toFirestore,
                    );
            DocumentReference docRef = await collectionRef.add(customDateTime);
            DocumentSnapshot<CustomDateTime> docSnap =
                await collectionRef.doc(docRef.id).get();
            print('Expected DateTime: $dateTime');
            print('Actual DateTime: ${docSnap.data()!.dateTime}');
          },
        ),
      ),
    );
  }
}

Additional context

This bug occurs while using either the emulator or a live Firebase project.

This bug does not occur when converting DateTime to a Timestamp beforehand: Timestamp.fromDate(customDateTime.dateTime). This can be seen by modifying the above example:

static Map<String, Object?> toFirestore(
  CustomDateTime customDateTime,
  SetOptions? options,
) {
  return <String, Object?>{
    'date': Timestamp.fromDate(customDateTime.dateTime),
  };
}

Example Inputs / Outputs

Expected DateTime: 2023-11-01 00:00:00.000001 . . .Actual DateTime: 2023-11-01 00:00:00.000

Expected DateTime: 2023-11-01 00:00:00.001001 . . .Actual DateTime: 2023-11-01 00:00:00.001

Expected DateTime: 2023-11-01 00:00:00.999999 . . .Actual DateTime: 2023-11-01 00:00:00.999

Expected DateTime: 2023-11-01 23:59:59.999999 . . .Actual DateTime: 2023-11-01 23:59:59.999

Expected DateTime: 2023-11-30 23:59:59.999999 . . .Actual DateTime: 2023-11-30 23:59:59.999


Flutter doctor

Run flutter doctor and paste the output below:

Click To Expand ``` [√] Flutter (Channel stable, 3.16.5, [redacted]) [√] [redacted] [√] Android toolchain - develop for Android devices (Android SDK version 32.0.0) [√] Chrome - develop for the web [√] Visual Studio - [redacted] (Visual Studio Community 2022 17.0.5) [√] Android Studio (version 2023.1) [√] IntelliJ IDEA Community Edition (version 2023.3) [√] VS Code (version 1.85.1) [√] Connected device ([redacted]) [√] Network resources • No issues found! ```

Flutter dependencies

Run flutter pub deps -- --style=compact and paste the output below:

Click To Expand ``` Dart SDK 3.2.3 Flutter SDK 3.16.5 dependencies: - cloud_firestore 4.13.6 - firebase_core 2.24.2 - flutter 0.0.0 ```

kwill39 commented 10 months ago

The sample code can be further simplified to exclude the use of a custom object.

import 'package:cloud_firestore/cloud_firestore.dart';
import 'package:firebase_core/firebase_core.dart';
import 'package:flutter/material.dart';

import 'firebase_options.dart';

void main() async {
  WidgetsFlutterBinding.ensureInitialized();
  await Firebase.initializeApp(
    options: DefaultFirebaseOptions.currentPlatform,
  );
  FirebaseFirestore firestore = FirebaseFirestore.instance;
  firestore.useFirestoreEmulator('localhost', 8080);
  Settings settings = const Settings(persistenceEnabled: false);
  firestore.settings = settings;
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: Scaffold(
        floatingActionButton: FloatingActionButton(
          onPressed: () async {
            int oneMicrosecond = 1;
            DateTime dateTime =
                DateTime(2023, 11, 1, 0, 0, 0, 0, oneMicrosecond);
            CollectionReference collectionRef =
                FirebaseFirestore.instance.collection('dates');
            DocumentReference docRef =
                await collectionRef.add({'date': dateTime});
            DocumentSnapshot docSnap = await collectionRef.doc(docRef.id).get();
            print('Expected DateTime: $dateTime');
            print(
                'Actual DateTime: ${((docSnap.data()! as Map<String, dynamic>)['date'] as Timestamp).toDate()}');
          },
        ),
      ),
    );
  }
}
darshankawar commented 10 months ago

Thanks for the detailed report @Region40 Although I am able to replicate the reported behavior, I am wondering if this is specific only while using cloud_firestore or not, or can also occur without it, just using DateTime class from Dart. Is there a way to confirm this ?

kwill39 commented 10 months ago

@darshankawar That's a good question. Yes, there is a way to confirm this. I removed both the cloud_firestore and firebase_core packages from dependencies and used only the Firebase rest api to interact with the emulator, and it appears the bug does not occur when using the rest api. This leads me to suspect that the issue results from how cloud_firestore implicitly converts a DateTime object. Here is a modified version of the previously provided code sample, along with the results of running the same inputs from earlier.

Please note that the code below assumes the project name is "demo-test". Please change the project name, in the code sample, from "demo-test" to the name of your project if you're not running the emulator using: firebase emulators:start --project demo-test.

If you're running on anything other than an Android device/emulator, replace 10.0.2.2 with localhost.

import 'dart:convert';
import 'dart:io';

import 'package:flutter/material.dart';

void main() async {
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: Scaffold(
        floatingActionButton: FloatingActionButton(
          onPressed: () async {
            final int oneMicrosecond = 1;
            final DateTime dateTime =
                DateTime(2023, 11, 1, 0, 0, 0, 0, oneMicrosecond);

            // Build the URL for the dates collection in the Firestore emulator
            final String collectionUrl = 'http://10.0.2.2:8080/v1/projects'
                '/demo-test/databases/(default)/documents/dates';
            Uri url = Uri.parse(collectionUrl);

            // Prepare a JSON string for a request to the Firestore emulator
            final String requestData = jsonEncode({
              'fields': {
                'timestampField': {
                  'timestampValue': dateTime.toUtc().toIso8601String()
                },
              },
            });

            HttpClient httpClient = HttpClient();
            try {
              // Add the document to the Firestore emulator
              HttpClientRequest request = await httpClient.postUrl(url);
              request.write(requestData);
              HttpClientResponse response = await request.close();
              String responseData =
                  await response.transform(utf8.decoder).join();
              String documentId =
                  jsonDecode(responseData)['name'].split('/').last;

              // Get the document
              url = Uri.parse('$collectionUrl/$documentId');
              request = await httpClient.getUrl(url);
              response = await request.close();
              responseData = await response.transform(utf8.decoder).join();
              String timestampString = jsonDecode(responseData)['fields']
                  ['timestampField']['timestampValue'];

              // Examine results
              print('Expected DateTime: $dateTime');
              print(
                  'Actual DateTime: ${DateTime.parse(timestampString).toLocal()}');
            } finally {
              httpClient.close();
            }
          },
        ),
      ),
    );
  }
}

Expected DateTime: 2023-11-01 00:00:00.000001 . . .Actual DateTime: 2023-11-01 00:00:00.000001

Expected DateTime: 2023-11-01 00:00:00.001001 . . .Actual DateTime: 2023-11-01 00:00:00.001001

Expected DateTime: 2023-11-01 00:00:00.999999 . . .Actual DateTime: 2023-11-01 00:00:00.999999

Expected DateTime: 2023-11-01 23:59:59.999999 . . .Actual DateTime: 2023-11-01 23:59:59.999999

Expected DateTime: 2023-11-30 23:59:59.999999 . . .Actual DateTime: 2023-11-30 23:59:59.999999

darshankawar commented 10 months ago

Thanks for the detailed update above @Region40 I think using both, TimeStamp and DateTime together might be a factor in the difference you are seeing. You are already converting the timestamp to date, which led me to this issue that you can check and see if it helps in anyway for your case or not.

kwill39 commented 10 months ago

@darshankawar

I think using both, TimeStamp and DateTime together might be a factor in the difference you are seeing.

That appears to be correct. Based on the examples provided above, it seems cloud_firestore does not accurately handle the conversion of DateTime.

The real concern is that cloud_firestore does not communicate this to the developer. It silently modifies the data as it converts a DateTime to Timestamp. This is unexpected and leaves the developer with inaccurate information in their database.

If cloud_firestore is not going to accurately handle the conversion of DateTime toTimestamp, then it needs to throw an error communicating to the developer that it cannot handle instances of DateTime. This is the current behavior that cloud_firestore adopts when handling data types that it cannot implicitly convert, such as DateTimeRange. This can be seen by modifying one of the examples above that uses cloud_firestore.

await collectionRef.add({'date': DateTimeRange(start: DateTime(2022), end: DateTime(2023))});
E/flutter (26606): [ERROR:flutter/runtime/dart_vm_initializer.cc(41)] Unhandled Exception: Invalid argument: Instance of 'DateTimeRange'
E/flutter (26606): #0      StandardMessageCodec.writeValue (package:flutter/src/services/message_codecs.dart:464:7)
E/flutter (26606): #1      FirestoreMessageCodec.writeValue (package:cloud_firestore_platform_interface/src/method_channel/utils/firestore_message_codec.dart:121:13)
. . .
nilsreichardt commented 10 months ago

I have a similar issue and I think it's the same bug. I have a feature in my app where I added pagination. So I have a first query (first page) and a second query (second page, uses startAfter). However, the last item of the first page is also displayed at the start of the second page. However, this only happens when the object in startAfter a DateTime is. Using Timestamp returns the expected result.

Query with Timestamp (returns correct result)

final snapshot = await firestore
    .collection('Sessions')
    .where(
      'userId',
      isEqualTo: userId,
    )
    .orderBy('createdAt', descending: true)
    .limit(deckListBatchSize)
    .get();
print('snapshot: ${snapshot.docs.map((d) => d.id)}');

final snapshot2 = await firestore
    .collection('Sessions')
    .where(
      'userId',
      isEqualTo: userId,
    )
    .orderBy('createdAt', descending: true)
    .limit(deckListBatchSize)
    .startAfter([snapshot.docs.last.get('createdAt')]).get();
print('snapshot: ${snapshot2.docs.map((d) => d.id)}');

Output:

First query:
1821910d-4beb-40e5-8af4-ef980ab92387,
a2084e2e-4059-4b77-b3b1-a30c2ee3759a,
0e041752-2ece-494c-b8bb-d60561e9e292,
78947fbc-a79e-4569-8d9c-8ddce0b6ad60,
142d4336-b1d6-4dd1-9037-d246cd547eda

Second query (with startAfter):
0177f085-af71-4041-9cd1-e3a47211bc52,
e485b50d-cb59-4c62-ae73-32fa590b0a4e,
3187d456-bc77-4037-b08b-0effab68ed78,
9089db8a-c17e-4974-a7dc-bb1e0fa7b155,
7feac8a1-91f1-4099-b693-553b4407a01a

Everything is as expected. No ids are duplicated.

Query with DateTime (returns wrong result)

final snapshot = await firestore
    .collection('Sessions')
    .where(
      'userId',
      isEqualTo: userId,
    )
    .orderBy('createdAt', descending: true)
    .limit(deckListBatchSize)
    .get();
print('snapshot: ${snapshot.docs.map((d) => d.id)}');

final snapshot2 = await firestore
    .collection('Sessions')
    .where(
      'userId',
      isEqualTo: userId,
    )
    .orderBy('createdAt', descending: true)
    .limit(deckListBatchSize)
    .startAfter([
  // Converting timestamp to date: this produces the bug
  (snapshot.docs.last.get('createdAt') as Timestamp).toDate()
]).get();

Output:

First query
1821910d-4beb-40e5-8af4-ef980ab92387,
a2084e2e-4059-4b77-b3b1-a30c2ee3759a,
0e041752-2ece-494c-b8bb-d60561e9e292,
78947fbc-a79e-4569-8d9c-8ddce0b6ad60,
142d4336-b1d6-4dd1-9037-d246cd547eda

Second query (with startAfter):
142d4336-b1d6-4dd1-9037-d246cd547eda,
0177f085-af71-4041-9cd1-e3a47211bc52,
e485b50d-cb59-4c62-ae73-32fa590b0a4e,
3187d456-bc77-4037-b08b-0effab68ed78,
9089db8a-c17e-4974-a7dc-bb1e0fa7b155

You can see that 142d4336-b1d6-4dd1-9037-d246cd547eda is at the end of the first query and at the start of the first query which shouldn't be the case when using startAfter().

More information

I can reproduce the issue using Flutter macOS. Using Flutter Web, everything works and the bug doesn't occur. I haven't tested it on Android.

Also writing a test in timestamp_test.dart doesn't reproduce the issue. Therefore, it's a problem in the integration of the native SDKs.

  test('temp', () {
      DateTime dateTime = DateTime(2023, 11, 1, 0, 0, 0, 0, 1);
      Timestamp t = Timestamp.fromDate(dateTime);

      print(dateTime);
      print(t.toDate());
      print(Timestamp.fromDate(dateTime).toDate());

      expect(t.toDate(), equals(dateTime));

      // This test passes.
    });
kwill39 commented 10 months ago

@nilsreichardt Thank you for verifying that the behavior is different depending on the platform. The tests I provided above were all run using an Android emulator, so we can add Android to the list of platforms affected by this bug.

darshankawar commented 10 months ago

Thanks for the update. Using the details provided (sample code) and running on Android, I do see slight difference in the date time output.