Add support for datetime properties #519
Conversation
| realm_value.ref.values.timestamp.seconds = microseconds ~/ 1000000; | ||
| realm_value.ref.values.timestamp.nanoseconds = (microseconds % 1000000) * 1000; | ||
| realm_value.ref.type = realm_value_type.RLM_TYPE_TIMESTAMP; | ||
| break; |
There was a problem hiding this comment.
So no timezone :-/ .. Was about to suggest using https://pub.dev/packages/timezone, but since core doesn't support there is little point.
There was a problem hiding this comment.
Yeah, that's something we've never supported and it comes up occasionally as an annoyance.
lib/src/native/realm_core.dart
Outdated
| throw Exception("Not implemented"); | ||
| final seconds = ref.values.timestamp.seconds; | ||
| final nanoseconds = ref.values.timestamp.nanoseconds; | ||
| return DateTime.fromMicrosecondsSinceEpoch(seconds * 1000000 + nanoseconds ~/ 1000).toUtc(); |
There was a problem hiding this comment.
Looks kosher, but I notice a lot of failing tests.
There was a problem hiding this comment.
Looks like a bug in date serialization in Core (or a bug in our code). Investigating.
test/realm_object_test.dart
Outdated
| }); | ||
|
|
||
| final json = obj.toJson(); | ||
| expect(json, contains('"dateProp":"${DateTime.utc(0).toRealmString()}"')); |
There was a problem hiding this comment.
| expect(json, contains('"dateProp":"${DateTime.utc(0).toRealmString()}"')); | |
| expect(json['dateprop'], DateTime.utc(0).toRealmString())); |
| @@ -265,3 +278,14 @@ Future<void> baasTest( | |||
| extension RealmObjectTest on RealmObject { | |||
| String toJson() => realmCore.objectToString(this); | |||
There was a problem hiding this comment.
This is surprising toJson by convention should return dynamic (runtime-type of objects will be Map<String, dynamic>). I would suggest running jsonDecode on the decoded String to follow convention.
There was a problem hiding this comment.
I'm slightly worried about taking such approach because the json Core produces might not be the most valid json in the world - particularly, they'll serialize integers as strings, dates as non-roundtrippable strings, etc.
| String toRealmString() { | ||
| final utc = toUtc(); | ||
| // This is kind of silly, but Core serializes negative dates as -003-01-01 12:34:56 | ||
| final utcYear = utc.year < 0 ? '-${utc.year.abs().toString().padLeft(3, '0')}' : utc.year.toString().padLeft(4, '0'); |
There was a problem hiding this comment.
Weird, and why pad 3? Just curious
There was a problem hiding this comment.
I have no idea - I believe it was likely overlooked by the core team.
|
I filed realm/realm-core#5451 to track fixes for roundtripping zero/negative dates. I tried it on Windows and the values look correct in Studio, so I'm 99% confident it's a serialization bug in |
|
@blagoev I think it would be nice to land this PR before release. |
|
This PR works around the issues with |
|
I was planning on doing that on top of master, but it can wait a bit to one of the next sprints |
|
Added in #569. |
|
reopening since this has nullable datetime support. |
|
@blagoev we already merged nullable datetime support (there was nothing special about this). Did you mean to reopen #462? Even so, we don't really need those PRs open - they'll still be accessible via the Github UI and we can't cherry pick them anyway since they've diverged too much from master. We'll probably need to update #459 to pull in oid and uuid support. |
nirinchev
force-pushed
the
ni/dynamic-realm
branch
from
065ac4f
to
09168af
Compare
nirinchev
force-pushed
the
ni/dynamic-realm
branch
from
5edab72
to
f773258
Compare
This depends on #495 because I was halfway through testing dynamic access when I realized dates are not supported.
Fixes #520
UPDATE: Superseded by #569. Kept as a draft cause it has nullable datetime support as well