[tor-bugs] #22836 [Metrics/Website]: Parse CollecTor's index.json and provide our own directory listing

Tor Bug Tracker & Wiki blackhole at torproject.org
Mon Sep 18 19:37:20 UTC 2017

#22836: Parse CollecTor's index.json and provide our own directory listing
 Reporter:  karsten          |          Owner:  karsten
     Type:  enhancement      |         Status:  needs_review
 Priority:  Medium           |      Milestone:
Component:  Metrics/Website  |        Version:
 Severity:  Normal           |     Resolution:
 Keywords:                   |  Actual Points:
Parent ID:                   |         Points:
 Reviewer:                   |        Sponsor:

Comment (by karsten):

 Replying to [comment:17 iwakeh]:
 > I cannot find a test for `static List<String[]> formatTableEntries`, did
 you forget a commit?

 I did not include one, because I didn't find an easy way to test this
 method. But I like your new test method and think it already covers this
 method quite well. Do you think we need to test more?

 > The waiting time shouldn't be extended unnecessarily:
 > {{{
 > -  } while (System.currentTimeMillis() < waitingSinceMillis + 10000L);
 > +  } while (null == this.index
 > +      && System.currentTimeMillis() < waitingSinceMillis + 10000L);
 > }}}

 Ouch. Yes, I meant to write that. Good catch!

 > In addition, it might be safer and shorter to simply use
 `AtomicReference` instead of the synchronized methods.

 Works for me.

 > Actually, I was hinting for a separate DirectoryListing class (in
 comment:14), but separating the retrieval of the index is important, too.
 > The separation you provided made it easy to introduce a
 `CollectorDirectoryProvider`, which fetches and provides the index incl.
 the suggested use of `AtomicReference` and the second while-condition.
 DirectoryListing is a `Map<String, List<String[]>>` and encapsulates the
 conversion from Index* to lists of strings.  `CollectorDirectoryProvider`
 only manages the scheduler and handing out the index.

 Looks good to me!

 > Another test `testListing` needs close inspection; it uses a fantasy
 index.json and checks the resulting map.

 Looks good!

 > Please, review the commit on top of
 web.git/commit/?h=task-22836 this branch].
 > (I didn't change the byte-calculations, b/c they are only 'decoration'
 and the compiler will make sure that a constant is created for the

 I only made a few trivial tweaks in [https://gitweb.torproject.org/karsten
 /metrics-web.git/log/?h=task-22836 my updated task-22836 branch] that is
 based on yours. If you like them, do you mind me squashing all commits
 into one before rebasing and pushing to master?

 Thanks for the review and improvements!

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

More information about the tor-bugs mailing list