MER-C / wiki-java

A MediaWiki bot framework in Java
GNU Affero General Public License v3.0
66 stars 58 forks source link

Truncate timestamps to microsecond precision #175

Closed PeterBowman closed 4 years ago

PeterBowman commented 4 years ago

This is a follow-up to https://github.com/MER-C/wiki-java/issues/170. I found a few more places where truncation is necessary, otherwise MW serves badtimestamp errors.

Suggestion: it is quite easy to mess up timestamp formatting upon conversion from OffsetDateTime into a string. Whereas #170 was an one-shot fix for all POST requests, which benefit from implicit Object-to-String conversion via convertToString, keep in mind that GET request params must be serialized by clients (cf. Map type parameters in getparams versus postparams in makeApiCall). Therefore, whenever client code forgets to truncate timestamps prior to converting them into strings, underlying makeApiCall calls might return a badtimestamp error. I observed that OffsetDateTime instances may produce different strings on different machines: my application ran fine on WMF servers, but crashed because of this issue on my local PC.

To sum up: perhaps the Map<String, String> getparams stuff could be refurbished into a Map<String, Object> getparamsand take advantage of convertToString as well?

Also, consider refactorization of:

odt.withOffsetSameInstant(ZoneOffset.UTC)
                    .truncatedTo(ChronoUnit.MICROS)
                    .format(DateTimeFormatter.ISO_OFFSET_DATE_TIME)

A protected method could be useful for derived classes.