-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Update Timestamp Constructor with nanoseconds #27
Merged
rrousselGit
merged 13 commits into
invertase:main
from
KKimj:@kkimj/update-Timestamp-with-nanoseconds
Jul 22, 2024
+84
−4
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
b6c1c99
feat: Update Timestamp Constructor with nanoseconds
KKimj 600fe77
Merge branch 'main' into @kkimj/update-Timestamp-with-nanoseconds
KKimj 3832b30
chore: Fix typo
KKimj 0c85dde
chore: Fix typo
KKimj 7a3e2b1
chore: Add tests
KKimj 1cf1a87
chore: update docstring
KKimj 45e0adb
Changelog
rrousselGit a8ee7e2
Swap
rrousselGit 8dbebc4
Format
rrousselGit 8a01ad4
fix: Fix error with Timestamp.fromMillis
KKimj 1be3baa
chore: Add tests
KKimj 8bc4fe7
chore: Remove white-space
KKimj 0bb4ee2
chore: Restore white-space
KKimj File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
47 changes: 47 additions & 0 deletions
47
packages/dart_firebase_admin/test/google_cloud_firestore/timestamp_test.dart
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
import 'package:dart_firebase_admin/firestore.dart'; | ||
import 'package:test/test.dart'; | ||
|
||
void main() { | ||
group('Timestamp', () { | ||
test('constructor', () { | ||
final now = DateTime.now().toUtc(); | ||
final seconds = now.millisecondsSinceEpoch ~/ 1000; | ||
final nanoseconds = | ||
(now.microsecondsSinceEpoch - seconds * 1000 * 1000) * 1000; | ||
|
||
expect( | ||
Timestamp(seconds: seconds, nanoseconds: nanoseconds), | ||
Timestamp.fromDate(now), | ||
); | ||
}); | ||
|
||
test('fromDate constructor', () { | ||
final now = DateTime.now().toUtc(); | ||
final timestamp = Timestamp.fromDate(now); | ||
|
||
expect(timestamp.seconds, now.millisecondsSinceEpoch ~/ 1000); | ||
}); | ||
|
||
test('fromMillis constructor', () { | ||
final now = DateTime.now().toUtc(); | ||
final timestamp = Timestamp.fromMillis(now.millisecondsSinceEpoch); | ||
|
||
expect(timestamp.seconds, now.millisecondsSinceEpoch ~/ 1000); | ||
expect( | ||
timestamp.nanoseconds, | ||
(now.millisecondsSinceEpoch % 1000) * (1000 * 1000), | ||
); | ||
}); | ||
|
||
test('fromMicros constructor', () { | ||
final now = DateTime.now().toUtc(); | ||
final timestamp = Timestamp.fromMicros(now.microsecondsSinceEpoch); | ||
|
||
expect(timestamp.seconds, now.microsecondsSinceEpoch ~/ (1000 * 1000)); | ||
expect( | ||
timestamp.nanoseconds, | ||
(now.microsecondsSinceEpoch % (1000 * 1000)) * 1000, | ||
); | ||
}); | ||
}); | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one needs a test too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done! Thanks : )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this constructor necessary? It makes sense to stick as close to the JavaScript implementation as possible imo, and I don't believe this exists there (could be wrong)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the Timestamp.fromMicros constructor is necessary. Considering the Dart native environment, the lack of this constructor would result in the loss of microsecond precision.
Additionally, while the microsecondsSinceEpoch property of the Dart DateTime API is available in the web environment, the limitations of JavaScript mean that there will still be a loss of precision in the microsecond unit.
Even though a code like
would not throw an error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, thanks for the insight. I'd much prefer the name to be
fromMicroseconds
, but leave that one to Remi to call :D