#28859: Use Java 8 date-time functionality in ExoneraTor ------------------------------------+---------------------- Reporter: karsten | Owner: karsten Type: enhancement | Status: assigned Priority: Medium | Milestone: Component: Metrics/ExoneraTor | Version: Severity: Normal | Keywords: Actual Points: | Parent ID: #23752 Points: | Reviewer: Sponsor: | ------------------------------------+---------------------- We had plans to use Java 8 date-time functionality in all our code bases for quite a while now. I finally picked this up by reading a few tutorials, mostly to figure out which of the three types we're going to use: `Instant`, `LocalDateTime`, or `ZonedDateTime`. I picked ExoneraTor for this discussion/review, because nothing else depends on it and we're currently not working on this code base in other tickets.
So, I think I have a plan now, and I'm presenting it here using snippets from my ExoneraTor patch: {{{ @@ -131,10 +132,11 @@ public class ExoneraTorDatabaseImporter { try { if (lockFile.exists()) { BufferedReader br = new BufferedReader(new FileReader(lockFile)); - long runStarted = Long.parseLong(br.readLine()); + Instant runStarted = Instant.ofEpochMilli(Long.parseLong( + br.readLine())); br.close(); - if (System.currentTimeMillis() - runStarted - < 6L * 60L * 60L * 1000L) { + if (runStarted.plus(Duration.ofHours(6L)) + .compareTo(Instant.now()) >= 0) { logger.warn("File 'exonerator-lock' is less than 6 " + "hours old. Exiting."); System.exit(1); }}} This code is used to write the current system time to a local lock file to determine when we need to obey that or can safely delete it. Let's ignore the design and just look at the date-time part for now. So, I think that we can replace code where we used `System.currentTimeMillis()` in the past with `Instant`. The concept is close enough to the "machine time" concept rather than "human time" that `Instant` will work for us. We care about hours here, not things like calendar months. {{{ @@ -223,7 +225,8 @@ public class ExoneraTorDatabaseImporter { /* Parse a consensus. */ private static void parseConsensus(RelayNetworkStatusConsensus consensus) { - long validAfterMillis = consensus.getValidAfterMillis(); + LocalDateTime validAfter = LocalDateTime.ofInstant(Instant.ofEpochMilli( + consensus.getValidAfterMillis()), ZoneOffset.UTC); for (NetworkStatusEntry entry : consensus.getStatusEntries().values()) { if (entry.getFlags().contains("Running")) { String fingerprintBase64 = null; }}} Things are a bit different here. The valid-after time in a consensus is not just a point on the timeline. It's an actual date and time that has a meaning for humans. That somewhat rules out the `Instant`. Now, why use `LocalDateTime` here und not `ZonedDateTime`, as suggested before? Well, I could imagine doing either, as long as we're consistent across our codebases. However, one reason that made me pick `LocalDateTime` in the end was the comparison with PostgreSQL's `TIMESTAMP [WITHOUT TIMEZONE]` vs. `TIMESTAMP WITH TIMEZONE` types. We're importing lots of timestamps into PostgreSQL, but we're always using `TIMESTAMP [WITHOUT TIMEZONE]`. Because we know that timestamps in Tor are always UTC and we simply don't need timezone information. The [https://jdbc.postgresql.org/documentation/head/8-date- time.html corresponding types] for `TIMESTAMP [WITHOUT TIMEZONE]` and `TIMESTAMP WITH TIMEZONE` are `LocalDateTime` and `OffsetDateTime`, by the way. The only use case I see where `LocalDateTime` could be problematic is when somebody wants to convert our timestamps to their local timezone. But that's not a common use case for us, and we're primarily building our tools for ourselves and the services we provide. {{{ @@ -248,26 +251,21 @@ public class ExoneraTorDatabaseImporter { [...] try { for (String orAddress : orAddresses) { insertStatusentryStatement.clearParameters(); - insertStatusentryStatement.setTimestamp(1, - new Timestamp(validAfterMillis), calendarUTC); + insertStatusentryStatement.setObject(1, validAfter); insertStatusentryStatement.setString(2, fingerprintBase64); if (!orAddress.contains(":")) { insertStatusentryStatement.setString(3, orAddress); }}} This is what it looks like when we're passing a `LocalDateTime` to the database. {{{ @@ -47,7 +50,9 @@ public class QueryServlet extends HttpServlet { private DataSource ds; [...] + private static final DateTimeFormatter validAfterTimeFormatter + = DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss") + .withZone(ZoneOffset.UTC); @Override public void init() { }}} And this is how we're parsing/formatting a Tor date-time. It's not a standard format, but we can easily define it once per class or package or codebase. Yay, thread safety! Okay, these are the highlights. I'll post the branch once I have a ticket number. -- Ticket URL: <https://trac.torproject.org/projects/tor/ticket/28859> Tor Bug Tracker & Wiki <https://trac.torproject.org/> The Tor Project: anonymity online
_______________________________________________ tor-bugs mailing list tor-bugs@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs