Flutterfire: Timestamp.fromMillisecondsSinceEpoch() throws when calculating negative milliseconds.

[marcglasberg]

Currently, this will throw an error:

Timestamp.fromMillisecondsSinceEpoch(-1);

I suggest the above code should be add as a test to the package.

It's a simple fix. The seconds/nanoseconds calculation is wrong:

    int milliseconds = -1;
    var _kThousand = 1000;
    var _kMillion = 1000000;

    // What is should be:
    int seconds = (milliseconds / _kThousand).floor();
    int nanoseconds = (milliseconds - seconds * _kThousand) * _kMillion;
    print('seconds = $seconds');
    print('nanoseconds = $nanoseconds');

    // Flutterfire's current code:
    seconds = (milliseconds ~/ _kThousand).floor();
    nanoseconds = (milliseconds - seconds * _kThousand) * _kMillion;
    print('seconds = $seconds');
    print('nanoseconds = $nanoseconds');

This is the same as https://github.com/flutter/flutter/issues/34929 and https://github.com/FirebaseExtended/flutterfire/issues/1222 which was not fixed after all.

This is a serious issue with an easy fix. Please, give it a high priority and don't let it stand for long. Thanks.

[Ehesp]

Hmm, I think asserting a valid millisecond value would be best here, the current code ensures you can't get an out of range exception on the high millisecond side.

[marcglasberg]

@Ehesp I respectfully disagree. The current code is just plain wrong. In the end you must have positive/negative seconds, but only positive nanoseconds. However, -1 is indeed a valid millisecond value for the function call. Negative milliseconds here are just dates before Jan 1st 1970, right?

[Ehesp]

I'll take a proper look at it t some point. Feel free to send a PR before that.

[marcglasberg]

Just to make it clear:

My original correction code was this: (It was wrong)

final int seconds = (milliseconds ~/ _kThousand);

Your correction code is this: (Also wrong, because it makes no sense to call floor after an integer division)

final int seconds = (milliseconds ~/ _kThousand).floor();

What is should be (Use floor, yes, to ensure you can't get an out of range exception on the high millisecond side. But first divide with doubles):

int seconds = (milliseconds / _kThousand).floor();

[dackers86]

Initial PR added at https://github.com/FirebaseExtended/flutterfire/pull/3565