[tor-bugs] #22279 [Metrics/metrics-lib]: simplify and avoid repetition in ParserHelper methods

[Tor Bug Tracker & Wiki]

#22279: simplify and avoid repetition in ParserHelper methods
---------------------------------+-----------------------------------
 Reporter:  iwakeh               |          Owner:  karsten
     Type:  enhancement          |         Status:  needs_review
 Priority:  Medium               |      Milestone:  metrics-lib 1.8.0
Component:  Metrics/metrics-lib  |        Version:
 Severity:  Normal               |     Resolution:
 Keywords:                       |  Actual Points:
Parent ID:                       |         Points:
 Reviewer:                       |        Sponsor:
---------------------------------+-----------------------------------

Comment (by iwakeh):

 Replying to [comment:14 karsten]:
 > Looks like we overlooked the case when a descriptor does not contain a
 line, e.g., `dirreq-v3-reqs`.  In this case,
 `ExtraInfoDescriptorImpl#dirreqV3Reqs` will stay `null`, and
 `ParseHelper#convertCommaSeparatedKeyIntegerValueList()` will not get a
 validated string from us but `null`.  Please find
 [https://gitweb.torproject.org/user/karsten/metrics-
 lib.git/commit/?h=task-22279-3&id=823fa496ce2b4f32f791fe7fd1859e67af068d3a
 this commit in my task-22279-3 branch] with a test and a fix.

 Good, that you found that!  Should there be a test added for this in
 particular?

 All looks fine, passes tests and checks.  I added a simple test class
 directly for DescriptorImpl [https://gitweb.torproject.org/user/iwakeh
 /metrics-
 lib.git/commit/?h=task-21932-3&id=991dc7bddfe6a6e692a3c580fe318ed01c23844d
 here].

 Some questions (partially not related to current changes):
 1. Rename `getScanner` into `newScanner`, because a new Scanner is created
 with every call and this is not a 'getter'?
 1. `ExitList.EOL = "\n" = DescriptorImpl.NL` why not use one for all
 delimiters in `newScanner` calls? Shouldn't `NL` be defined in
 `Descriptor` and be used everywhere, i.e., deprecate `ExitList.EOL` and
 replace with release 2.0.0?
 1. Make NL the delimiter default that is already set for the Scanner
 returned by `newScanner`?
 1. The Scanner usage in TorperfResult is essentially getLine() in the two
 places.  Do Torperf descriptors contain "\r"?
 1. DescriptorImpl.setDigestXXX allow empty or null argument.  Should there
 be a check?

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