[tor-bugs] #16151 [metrics-lib]: Add new descriptor source to fetch descriptors from CollecTor via https

[Tor Bug Tracker & Wiki]

#16151: Add new descriptor source to fetch descriptors from CollecTor via https
-----------------------------+--------------------------
     Reporter:  karsten      |      Owner:  karsten
         Type:  enhancement  |     Status:  needs_review
     Priority:  normal       |  Milestone:
    Component:  metrics-lib  |    Version:
   Resolution:               |   Keywords:
Actual Points:               |  Parent ID:
       Points:               |
-----------------------------+--------------------------

Comment (by iwakeh):

 Is there a task already for switching to java 7 and adding some logging
 framework?
 These two comprise my "ceterum censeo ..." :-)
 In general, could this change be also used to softly introduce some
 logging into metrics-lib?

 Now the code comments/questions:
 (I hope I didn't miss any new test cases ...)
 It might be helpful to add some tests for DescriptorCollectorImpl that
 explicit test for certain behavior.
 These would be a very good source of documentation: e.g. take the comment
 inside "putTogetherHistory". A test could explicitly verify the
 functionality
 described in the comments.

 DescriptorCollector interface:
 Why not turn the comments into javadoc? They are there already and very
 readable. Just adding the tags for return values and parameters is not a
 big deal.

 Concerning DescriptorCollectorImpl:
 Let's try to avoid the TODOs here from the beginning ;-)
 For the various constants (COLLECTOR_BASE_URL, CONNECT_TIMEOUT_MILLIS,
 READ_TIMEOUT_MILLIS)
 I would suggest using a system property (defaulting to the current values)
 as for the class
 names in DescriptorSourceFactory using
 {{{
 onionoo.collector.url
 onionoo.collector.timeout.read
 onionoo.collector.timeout.write
 }}}
 as property names. Then it will be configurable immediately.
 The retrieval of the 'int' properties needs just one extra method that
 returns
 either the converted property value or logs an error and returns the
 default.
 The URL could also be verified, if there is time to implement it now.

 The other TODO is the Apache's directory list parsing: the regex parsing
 approach is straigthforward and ok, but I'd make it testable and definitly
 log any problems. So a method 'addDirsFromListing(SortedMap<String, Long>
 list)'
 with the code from lines 177-200 and the additional logging might be
 useful.

 Well, and the test for parsing directory string needs to be added.
 Tests for the property handling above would be helpful, too.

 What about adding the {{{@Override}}} annotation where appropriate?

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