commit 51d883ba06bed1342311c8d8b78a8e6aea70f043 Author: Karsten Loesing <karsten.loesing@gmx.net> Date: Mon Apr 21 11:12:08 2014 +0200 Don't alter NodeStatus in DetailsDocumentWriter. The following two things went wrong when splitting up NodeDataWriter into two classes in e152b67: - Writing exit addresses to details documents doesn't require adding these addresses to the NodeStatus instance only to filter out known OR addresses. That's something we can also do in DetailsDocumentWriter. - Writing details documents is not the right place to learn the latest contact line of a relay to update our NodeStatus. This is something we can do when parsing the relay descriptor in NodeDetailsStatusUpdater. These are no bugs, but things that make it harder to refactor DetailsDocumentWriter. --- .../torproject/onionoo/DetailsDocumentWriter.java | 33 +++++++------------- .../onionoo/NodeDetailsStatusUpdater.java | 18 ++++++++--- 2 files changed, 26 insertions(+), 25 deletions(-) diff --git a/src/org/torproject/onionoo/DetailsDocumentWriter.java b/src/org/torproject/onionoo/DetailsDocumentWriter.java index 1682d7d..950a332 100644 --- a/src/org/torproject/onionoo/DetailsDocumentWriter.java +++ b/src/org/torproject/onionoo/DetailsDocumentWriter.java @@ -244,25 +244,29 @@ public class DetailsDocumentWriter implements DescriptorListener, /* Add exit addresses if at least one of them is distinct from the * onion-routing addresses. */ - if (exitListEntries.containsKey(fingerprint)) { + SortedSet<String> exitAddresses = new TreeSet<String>(); + if (this.exitListEntries.containsKey(fingerprint)) { for (ExitListEntry exitListEntry : - exitListEntries.get(fingerprint)) { - entry.addExitAddress(exitListEntry.getExitAddress()); + this.exitListEntries.get(fingerprint)) { + String exitAddress = exitListEntry.getExitAddress(); + if (exitAddress.length() > 0 && + !entry.getAddress().equals(exitAddress) && + !entry.getOrAddresses().contains(exitAddress)) { + exitAddresses.add(exitAddress); + } } } - if (!entry.getExitAddresses().isEmpty()) { + if (!exitAddresses.isEmpty()) { sb.append(",\n\"exit_addresses\":["); int written = 0; - for (String exitAddress : entry.getExitAddresses()) { + for (String exitAddress : exitAddresses) { sb.append((written++ > 0 ? "," : "") + "\"" + exitAddress.toLowerCase() + "\""); } sb.append("]"); } - /* Append descriptor-specific part from details status file, and - * update contact in node status. */ - /* TODO Updating the contact here seems like a pretty bad hack. */ + /* Append descriptor-specific part from details status file. */ DetailsStatus detailsStatus = this.documentStore.retrieve( DetailsStatus.class, false, fingerprint); if (detailsStatus != null && @@ -275,18 +279,9 @@ public class DetailsDocumentWriter implements DescriptorListener, if (line.startsWith("\"desc_published\":")) { continue; } - if (line.startsWith("\"contact\":")) { - int start = "\"contact\":\"".length(), - end = line.length() - 1; - if (line.endsWith(",")) { - end--; - } - contact = unescapeJSON(line.substring(start, end)); - } sb.append("\n" + line); } s.close(); - entry.setContact(contact); } /* Finish details string. */ @@ -364,10 +359,6 @@ public class DetailsDocumentWriter implements DescriptorListener, return StringEscapeUtils.escapeJavaScript(s).replaceAll("\\\\'", "'"); } - private static String unescapeJSON(String s) { - return StringEscapeUtils.unescapeJavaScript(s.replaceAll("'", "\\'")); - } - public String getStatsString() { /* TODO Add statistics string. */ return null; diff --git a/src/org/torproject/onionoo/NodeDetailsStatusUpdater.java b/src/org/torproject/onionoo/NodeDetailsStatusUpdater.java index 4d32b2c..4cfdd33 100644 --- a/src/org/torproject/onionoo/NodeDetailsStatusUpdater.java +++ b/src/org/torproject/onionoo/NodeDetailsStatusUpdater.java @@ -37,6 +37,8 @@ public class NodeDetailsStatusUpdater implements DescriptorListener, private SortedMap<String, NodeStatus> knownNodes = new TreeMap<String, NodeStatus>(); + private Map<String, String> contacts = new HashMap<String, String>(); + private SortedMap<String, NodeStatus> relays; private SortedMap<String, NodeStatus> bridges; @@ -99,8 +101,8 @@ public class NodeDetailsStatusUpdater implements DescriptorListener, Logger.printStatusTime("Started reverse domain name lookups"); this.lookUpCitiesAndASes(); Logger.printStatusTime("Looked up cities and ASes"); - this.setRunningBits(); - Logger.printStatusTime("Set running bits"); + this.setRunningBitsAndContacts(); + Logger.printStatusTime("Set running bits and contacts"); this.calculatePathSelectionProbabilities(); Logger.printStatusTime("Calculated path selection probabilities"); this.finishReverseDomainNameLookups(); @@ -209,8 +211,10 @@ public class NodeDetailsStatusUpdater implements DescriptorListener, } } - private void setRunningBits() { - for (NodeStatus node : this.knownNodes.values()) { + private void setRunningBitsAndContacts() { + for (Map.Entry<String, NodeStatus> e : this.knownNodes.entrySet()) { + String fingerprint = e.getKey(); + NodeStatus node = e.getValue(); if (node.isRelay() && node.getRelayFlags().contains("Running") && node.getLastSeenMillis() == this.relaysLastValidAfterMillis) { node.setRunning(true); @@ -219,6 +223,9 @@ public class NodeDetailsStatusUpdater implements DescriptorListener, node.getLastSeenMillis() == this.bridgesLastPublishedMillis) { node.setRunning(true); } + if (this.contacts.containsKey(fingerprint)) { + node.setContact(this.contacts.get(fingerprint)); + } } } @@ -363,6 +370,9 @@ public class NodeDetailsStatusUpdater implements DescriptorListener, detailsStatus = new DetailsStatus(); detailsStatus.setDocumentString(sb.toString()); this.documentStore.store(detailsStatus, fingerprint); + if (descriptor.getContact() != null) { + this.contacts.put(fingerprint, descriptor.getContact()); + } } private void processBridgeServerDescriptor(