[tor-bugs] #16596 [Metrics/ExoneraTor]: Change database queries towards making only a single query per request

[Tor Bug Tracker & Wiki]

#16596: Change database queries towards making only a single query per request
--------------------------------+------------------------------
 Reporter:  karsten             |          Owner:  karsten
     Type:  enhancement         |         Status:  needs_review
 Priority:  Medium              |      Milestone:
Component:  Metrics/ExoneraTor  |        Version:
 Severity:  Normal              |     Resolution:
 Keywords:                      |  Actual Points:
Parent ID:                      |         Points:
 Reviewer:                      |        Sponsor:
--------------------------------+------------------------------

Comment (by iwakeh):

 Already found a bit.  There are also a few things that are part of other
 tickets (#19623, #19624, #21145), which I tried to omit here.

 ExoneraTorServlet: `exoneratorHost` should not be configured via web.xml,
 rather use simple java properties file or even simpler hard code.  After
 all it shouldn't change that often, should it?

 In 'step 3' I see some problems with `null` values, for example,
 `"".equals(null)` would evaluate to false (line 147 following).

 For most exceptions caught the error message should be logged; and, it
 might be time to switch to slf4j-api and logback, now.

 In addition, reading the response query should also catch
 RuntimeExceptions (possibly from Gson).

 The version received should also be logged, if it differs from the known
 version.
 The known version(s) could be a constant in `ExoneraTorServlet`; either
 just the major part or all.  This makes it more obvious.

 QueryServlet: Helper methods ` private String parseTimestampParameter` and
 `private String parseIpParameter` should be `public static` and tests
 should be added.  Similarly, `private String convertIpV*ToHex`, which
 could be combined by simply adding another argument, i.e., `public static
 String convertIpV4ToHex(String relayIp, boolean isIpV4)`.
 A `MILLISEC_IN_DAY = 24L * 60L * 60L * 1000L` constant would be useful.


 Other: Using the object `Boolean exit` for a triplet state is a bit too
 much of a hack.  There could be a simple enum, as simple as, for example,
 `public enum Exit { U, Y, N; }`.

 Regarding SQL:
 Order-by statement should be using names not numbers.
 Couldn't the exit-information be part of the query already?

 (I could also look into providing some patches, if we agree on the changes
 and you don't have the time?)

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/16596#comment:6>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online