[tor-bugs] #20049 [Metrics/Metrics website]: Adapt legacy module to accept bridge network statuses from two authorities

[Tor Bug Tracker & Wiki]

#20049: Adapt legacy module to accept bridge network statuses from two authorities
-------------------------------------+-----------------------------
 Reporter:  karsten                  |          Owner:
     Type:  defect                   |         Status:  merge_ready
 Priority:  Very High                |      Milestone:
Component:  Metrics/Metrics website  |        Version:
 Severity:  Normal                   |     Resolution:
 Keywords:                           |  Actual Points:
Parent ID:                           |         Points:
 Reviewer:                           |        Sponsor:
-------------------------------------+-----------------------------
Changes (by iwakeh):

 * status:  needs_review => merge_ready


Comment:

 Some findings below; this is difficult to review b/c of the ancient code
 base.
 I assume it does what is intended.

 * build.xml still references descriptor-1.2.0

 * It could be useful to consider enums for authority listings and EnumMap,
 as these might be faster than hash-maps, but ok for the moment.

 * In the existing code `bw.append(line + "\n");` should be replaced by the
 two statements  `bw.append(line); bw.newLine();`.

 * I would place a comment differently and leave out the empty else.  Minor
 change suggestion w/o enum change:
 {{{
 diff --git
 a/modules/legacy/src/org/torproject/ernie/cron/network/ConsensusStatsFileHandler.java
 b/modules/legacy/src/org/torproject/ernie/cron/network/ConsensusStatsFileHandler.java
 index 631839d..8d51d5d 100644
 ---
 a/modules/legacy/src/org/torproject/ernie/cron/network/ConsensusStatsFileHandler.java
 +++
 b/modules/legacy/src/org/torproject/ernie/cron/network/ConsensusStatsFileHandler.java
 @@ -139,6 +139,8 @@ public class ConsensusStatsFileHandler {
                  + "! Aborting to read this file!");
              break;
            }
 +          /* Assume that all lines without authority nickname are based
 on
 +           * Tonga's network status, not Bifroest's. */
            String key = parts[0] + "," + (parts.length < 4 ? "Tonga" :
 parts[1]);
            String value = null;
            if (parts.length == 2) {
 @@ -147,11 +149,8 @@ public class ConsensusStatsFileHandler {
              value = key + "," + parts[1] + "," + parts[2];
            } else if (parts.length == 4) {
              value = key + "," + parts[2] + "," + parts[3];
 -          } else {
 -            /* Impossible, we already checked the range above. */
 -          }
 -          /* Assume that all lines without authority nickname are based
 on
 -           * Tonga's network status, not Bifroest's. */
 +          } /* No more cases as we already checked the range above. */
 +
            this.bridgesRaw.put(key, value);
          }
          br.close();
 @@ -308,7 +307,8 @@ public class ConsensusStatsFileHandler {
            new FileWriter(this.bridgeConsensusStatsRawFile));
        bw.append("datetime,authority,brunning,brunningec2\n");
        for (String line : this.bridgesRaw.values()) {
 -        bw.append(line + "\n");
 +        bw.append(line);
 +        bw.newLine();
        }
        bw.close();
        this.logger.fine("Finished writing file "
 @@ -404,7 +404,7 @@ public class ConsensusStatsFileHandler {
                + "old: " + this.bridgesRaw.lastKey());
          }
        } catch (ParseException e) {
 -         /* Can't parse the timestamp? Whatever. */
 +        logger.warning("Can't parse the timestamp? Reason: " + e);
        }
      }
      logger.info(dumpStats.toString());
 }}}


 ------
 This module should be refactored very soon! But, that's a different
 ticket.

 Should there also be a follow-up ticket for accommodating future authority
 changes/additions more elegantly?

 Ready for merge as hot-fix.

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