Parse ISO 8601 date(time) strings as Instant

[PattaFeuFeu]

Description

The Mastodon API returns date time properties in quite a lot of method return types / entities.
We parse as many other properties as we can into our own data types so this PR attempts to also parse date (time) strings into instances of Instant where possible.

This closes #311.

java.time vs kotlinx.time

For a bit I thought about using https://github.com/Kotlin/kotlinx-datetime but decided against it as that would mean a lot of translating from java.time to kotlinx.time and vice versa if consumers of BigBone are using java.time only and would like to keep it that way.

It would have made the custom (de-)serializer obsolete but for the previously stated reason, I decided against it.

ISO 8601 date time string vs LocalDate string

The reports API method have an example with "last_status_at": "2022-08-25", i.e., a LocalDate, but endorsements, also referencing Account in the example have "last_status_at": "2019-11-23T07:05:50.682Z", i.e. a full ISO 8601 date time at UTC.

It wasn’t clear to me if both could always be possible so I opted for a solution where I first try to parse an Instant from an ISO 8601 Instant string and if that fails, I fall back to a LocalDate date formatter. If the LocalDate variant succeeds, I get that LocalDate and return an Instant at the start of that day on a UTC timeline.

Default values

I wasn’t sure what a good equivalent of the previous default "" could be. At first I had implemented it as Instant.ofEpochSecond(0) but that also didn’t seem quite right: If consumers of BigBone would receive an Instant at 1970-01-01T00:00:00Z, it would seem like a potentially valid value the API returned after a call.

For that reason, I have added all values that do not mention “optional“ on their documentation page simple as : Instant without a default value supplied.
I made one exception here: Account doesn’t have created_at as “optional” but we have recently added JSON example files where created_at is not available, so I put it as nullable, just in case.

Decision needed: Should we make all Instants nullable, just to have a default value? Should we supply a different, non-null default value? If so: which one?

Recent change with 1700787: I have added the concept of “Precision” at the suggestion of @bocops. Instead of directly returning an Instant now, we now return PrecisionDateTime which can be one of ExactTime, StartOfDay, Invalid, or Unavailable.

The default value is set so Unavailable everywhere. When attempting to deserialise the JSON we get from the Mastodon API, we first try to parse an ExactTime, fall back to StartOfDay on error, and if that also throws a parsing exception, we fall back to Invalid, passing the exception to make checking what was going wrong easier.

Type of Change

(Keep the one that applies, remove the rest)

• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Types previously returned as String are now Instant or Instant? PrecisionDateTime instances.
  • Methods that previously expected a String for scheduling now expect an Instant instead
• This change requires a documentation update (maybe?)

How Has This Been Tested?

I’ve updated existing tests for methods with dates in their JSON and I’ve added new tests for happy and unhappy paths of the custom deserializer.

Mandatory Checklist

• My change follows the projects coding style
• I ran gradle check and there were no errors reported
• I have performed a self-review of my code
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[codecov]

Codecov Report

Merging #312 (5df6069) into master (b42c060) will increase coverage by 0.33%.
The diff coverage is 69.09%.

Additional details and impacted files

@@             Coverage Diff              @@
##             master     #312      +/-   ##
============================================
+ Coverage     45.08%   45.42%   +0.33%     
- Complexity      422      428       +6     
============================================
  Files           112      113       +1     
  Lines          3012     3049      +37     
  Branches        201      203       +2     
============================================
+ Hits           1358     1385      +27     
- Misses         1511     1521      +10     
  Partials        143      143              

Files	Coverage Δ
...c/main/kotlin/social/bigbone/rx/RxStatusMethods.kt	0.00% <ø> (ø)
...e/src/main/kotlin/social/bigbone/JsonSerializer.kt	100.00% <100.00%> (ø)
.../main/kotlin/social/bigbone/api/entity/Instance.kt	63.47% <100.00%> (ø)
...rc/main/kotlin/social/bigbone/api/entity/Marker.kt	44.44% <100.00%> (ø)
...n/kotlin/social/bigbone/api/entity/Notification.kt	25.00% <100.00%> (ø)
...rc/main/kotlin/social/bigbone/api/entity/Report.kt	40.74% <100.00%> (+7.40%)	⬆️
...rc/main/kotlin/social/bigbone/api/entity/Status.kt	85.52% <100.00%> (ø)
...ain/kotlin/social/bigbone/api/entity/StatusEdit.kt	27.58% <100.00%> (ø)
.../kotlin/social/bigbone/api/method/StatusMethods.kt	75.25% <100.00%> (+0.65%)	⬆️
...c/main/kotlin/social/bigbone/api/entity/Account.kt	85.48% <66.66%> (ø)
... and 5 more

[bocops]

This is a good idea. I'll follow this up with a review, but wanted to mention some things in advance.

First, what you suggest is exactly how I'm currently doing it in my app using BigBone: try to parse as a "precise" timestamp, then fall back to an "imprecise" one if that fails. This has worked well for me so far - but having the library provide the parsing would of course be better.

Second, regarding Mastodon's return values, some of the imprecise values are apparently returned for privacy reasons, and that is probably where the difference in our test data is coming from. The last_posted_at and created_at values of Account were changed from precise to imprecise with Mastodon 3.1.0 and 3.4.0 respectively, for example, and we (or Mastodon docs themselves) just may have outdated examples.

Some endpoints even return something like the "Unix timestamp on midnight of the given day", we might want to deal with that as well, either now or later.

We should definitely be fault-tolerant in the different types of values that might be returned by any of the endpoints.

Last but not least, I think the details matter when it comes to turning all of them into seemingly precise timestamps/Instants, especially if the original value was invalid, but potentially also when it was just imprecise.

For example, one use case of my app is to periodically check the last_posted_at value of followed accounts, to automatically unfollow those that have been inactive for a while. If, during this process, this value suddenly is equal to early 1970 just because it could not be parsed, it might lead to accounts being unfollowed although they are still active. I definitely would want to know the difference between a genuinely "old" timestamp and an invalid one in this case, but might be happy with just comparing to any possible default value in other cases.

How about returning instances of a data class that contains an Instant and a Precision enum (with values like Precision.INSTANT, Precision.DAY, Precision.INVALID, exact names to be discussed)? Library users could then simply use the Instant where details don't matter to them, but could check the underlying precision if they need to.

[PattaFeuFeu]

How about returning instances of a data class that contains an Instant and a Precision enum (with values like Precision.INSTANT, Precision.DAY, Precision.INVALID, exact names to be discussed)? Library users could then simply use the Instant where details don't matter to them, but could check the underlying precision if they need to.

I like that idea! I will implement that and add another commit here with a proposal.

Thanks for your thoughts! Good to see that we apparently have/had the same ideas here.

[bocops]

Looks good. Happy to approve after we've decided on the idea to make information about the actual precision of a returned value (date+time vs. just date) available to consumers one way or the other.

If we do, we might not need nullable fields at least in some cases.

[bocops]

The documentation for Entity fields that were cheanged to the new PrecisionDateTime type should no longer claim that they are ISO 8601 date or datetime Strings, and/or that they can be null.

Other than this Kdoc change, I'm very happy with how this turned out. Thanks a lot. :)

[PattaFeuFeu]

@bocops I’ve updated the outdated kDocs.

I also noticed a few other issues I had not yet noticed before and have updated the PR accordingly:

• Serialize invalid values as null – We currently do not need the serializer but only the deserializer. But I wanted to make sure that the serializer would return expected results should we ever need it in the future.
• Expect scheduledAt as Instant instead of as String – I only noticed when searching for 8601 and At: String in the project that I was missing methods that have a String as a method parameter. I have replaced those with Instant now.
• Introduce client-side validation on methods #293 Add client-side validation for scheduling date – While I was at it, I also added client-side validation for those so that we fail early before starting a network request if the scheduled date is not far ahead enough.

Done now!

[bocops]

Looks good now! :)

[andregasser]

@PattaFeuFeu @bocops I'd like to review this code before it is merged into master. Thx.