[tor-commits] [onionoo/master] Avoid System.currentTimeMillis() to facilitate testing.

commit 515797ca889db67c98bf287db0e3fc25acba4640 Author: Karsten Loesing karsten.loesing@gmx.net Date: Mon Nov 25 08:50:46 2013 +0100

Avoid System.currentTimeMillis() to facilitate testing. --- .../torproject/onionoo/BandwidthDataWriter.java | 16 ++++++------- src/org/torproject/onionoo/DocumentStore.java | 11 +++++---- src/org/torproject/onionoo/LockFile.java | 7 ++++-- src/org/torproject/onionoo/Logger.java | 25 ++++++++++++++++---- src/org/torproject/onionoo/Main.java | 15 ++++++------ src/org/torproject/onionoo/NodeDataWriter.java | 8 ++++--- src/org/torproject/onionoo/ResourceServlet.java | 11 +++++---- src/org/torproject/onionoo/ResponseBuilder.java | 10 ++++---- .../onionoo/ReverseDomainNameResolver.java | 14 +++++++---- src/org/torproject/onionoo/Time.java | 14 +++++++++++ src/org/torproject/onionoo/WeightsDataWriter.java | 6 +++-- .../torproject/onionoo/ResourceServletTest.java | 12 +++++++++- 12 files changed, 104 insertions(+), 45 deletions(-)

diff --git a/src/org/torproject/onionoo/BandwidthDataWriter.java b/src/org/torproject/onionoo/BandwidthDataWriter.java index 8a0ccdf..6a96b98 100644 --- a/src/org/torproject/onionoo/BandwidthDataWriter.java +++ b/src/org/torproject/onionoo/BandwidthDataWriter.java @@ -40,18 +40,19 @@ public class BandwidthDataWriter implements DescriptorListener {

private DocumentStore documentStore;

- public BandwidthDataWriter(DescriptorSource descriptorSource, - DocumentStore documentStore) { - this.descriptorSource = descriptorSource; - this.documentStore = documentStore; - this.registerDescriptorListeners(); - } + private long now;

private SimpleDateFormat dateTimeFormat = new SimpleDateFormat( "yyyy-MM-dd HH:mm:ss"); - public BandwidthDataWriter() { + + public BandwidthDataWriter(DescriptorSource descriptorSource, + DocumentStore documentStore, Time time) { + this.descriptorSource = descriptorSource; + this.documentStore = documentStore; + this.now = time.currentTimeMillis(); this.dateTimeFormat.setLenient(false); this.dateTimeFormat.setTimeZone(TimeZone.getTimeZone("UTC")); + this.registerDescriptorListeners(); }

private void registerDescriptorListeners() { @@ -163,7 +164,6 @@ public class BandwidthDataWriter implements DescriptorListener { } }

- private long now = System.currentTimeMillis(); private void compressHistory( SortedMap<Long, long[]> history) { SortedMap<Long, long[]> uncompressedHistory = diff --git a/src/org/torproject/onionoo/DocumentStore.java b/src/org/torproject/onionoo/DocumentStore.java index d96d1c5..b78f5ff 100644 --- a/src/org/torproject/onionoo/DocumentStore.java +++ b/src/org/torproject/onionoo/DocumentStore.java @@ -33,6 +33,8 @@ public class DocumentStore {

private File outDir;

+ private Time time; + boolean listedArchivedNodeStatuses = false, listedCurrentNodeStatuses = false;

@@ -46,13 +48,14 @@ public class DocumentStore { * operations depend on which NodeStatus documents were listed. */ private SortedMap<String, NodeStatus> cachedNodeStatuses;

- public DocumentStore(File outDir) { - this.outDir = outDir; + public DocumentStore(File outDir, Time time) { + this(null, outDir, time); }

- public DocumentStore(File statusDir, File outDir) { + public DocumentStore(File statusDir, File outDir, Time time) { this.statusDir = statusDir; this.outDir = outDir; + this.time = time; }

public <T extends Document> SortedSet<String> list( @@ -473,7 +476,7 @@ public class DocumentStore { return; } File updateFile = new File(this.outDir, "update"); - String documentString = String.valueOf(System.currentTimeMillis()); + String documentString = String.valueOf(this.time.currentTimeMillis()); try { updateFile.getParentFile().mkdirs(); BufferedWriter bw = new BufferedWriter(new FileWriter(updateFile)); diff --git a/src/org/torproject/onionoo/LockFile.java b/src/org/torproject/onionoo/LockFile.java index aad7076..acf2dc2 100644 --- a/src/org/torproject/onionoo/LockFile.java +++ b/src/org/torproject/onionoo/LockFile.java @@ -11,8 +11,11 @@ public class LockFile {

private File lockFile;

- public LockFile(File lockFile) { + private Time time; + + public LockFile(File lockFile, Time time) { this.lockFile = lockFile; + this.time = time; }

public boolean acquireLock() { @@ -25,7 +28,7 @@ public class LockFile { } BufferedWriter bw = new BufferedWriter(new FileWriter( this.lockFile)); - bw.append("" + System.currentTimeMillis() + "\n"); + bw.append("" + this.time.currentTimeMillis() + "\n"); bw.close(); return true; } catch (IOException e) { diff --git a/src/org/torproject/onionoo/Logger.java b/src/org/torproject/onionoo/Logger.java index 7248ee6..fa113f3 100644 --- a/src/org/torproject/onionoo/Logger.java +++ b/src/org/torproject/onionoo/Logger.java @@ -5,6 +5,21 @@ package org.torproject.onionoo; import java.util.Date;

public class Logger { + + private static Time time; + + public static void setTime(Time timeParam) { + time = timeParam; + } + + private static long currentTimeMillis() { + if (time == null) { + return System.currentTimeMillis(); + } else { + return time.currentTimeMillis(); + } + } + public static String formatDecimalNumber(long decimalNumber) { return String.format("%,d", decimalNumber); } @@ -24,12 +39,11 @@ public class Logger { } }

- private static long printedLastStatusMessage = - System.currentTimeMillis(); + private static long printedLastStatusMessage = -1L;

public static void printStatus(String message) { System.out.println(new Date() + ": " + message); - printedLastStatusMessage = System.currentTimeMillis(); + printedLastStatusMessage = currentTimeMillis(); }

public static void printStatistics(String component, String message) { @@ -37,8 +51,9 @@ public class Logger { }

public static void printStatusTime(String message) { - long now = System.currentTimeMillis(); - long millis = now - printedLastStatusMessage; + long now = currentTimeMillis(); + long millis = printedLastStatusMessage < 0 ? 0 : + now - printedLastStatusMessage; System.out.println(" " + message + " (" + Logger.formatMillis(millis) + ")."); printedLastStatusMessage = now; diff --git a/src/org/torproject/onionoo/Main.java b/src/org/torproject/onionoo/Main.java index a62918b..84e7e15 100644 --- a/src/org/torproject/onionoo/Main.java +++ b/src/org/torproject/onionoo/Main.java @@ -8,9 +8,10 @@ import java.io.File; public class Main { public static void main(String[] args) {

+ Time t = new Time(); + LockFile lf = new LockFile(new File("lock"), t); + Logger.setTime(t); Logger.printStatus("Initializing."); - - LockFile lf = new LockFile(new File("lock")); if (lf.acquireLock()) { Logger.printStatusTime("Acquired lock"); } else { @@ -23,17 +24,17 @@ public class Main { new File("status")); Logger.printStatusTime("Initialized descriptor source"); DocumentStore ds = new DocumentStore(new File("status"), - new File("out")); + new File("out"), t); Logger.printStatusTime("Initialized document store"); LookupService ls = new LookupService(new File("geoip")); Logger.printStatusTime("Initialized Geoip lookup service"); - ReverseDomainNameResolver rdnr = new ReverseDomainNameResolver(); + ReverseDomainNameResolver rdnr = new ReverseDomainNameResolver(t); Logger.printStatusTime("Initialized reverse domain name resolver"); - NodeDataWriter ndw = new NodeDataWriter(dso, rdnr, ls, ds); + NodeDataWriter ndw = new NodeDataWriter(dso, rdnr, ls, ds, t); Logger.printStatusTime("Initialized node data writer"); - BandwidthDataWriter bdw = new BandwidthDataWriter(dso, ds); + BandwidthDataWriter bdw = new BandwidthDataWriter(dso, ds, t); Logger.printStatusTime("Initialized bandwidth data writer"); - WeightsDataWriter wdw = new WeightsDataWriter(dso, ds); + WeightsDataWriter wdw = new WeightsDataWriter(dso, ds, t); Logger.printStatusTime("Initialized weights data writer");

Logger.printStatus("Reading descriptors."); diff --git a/src/org/torproject/onionoo/NodeDataWriter.java b/src/org/torproject/onionoo/NodeDataWriter.java index 480f18b..64176c3 100644 --- a/src/org/torproject/onionoo/NodeDataWriter.java +++ b/src/org/torproject/onionoo/NodeDataWriter.java @@ -43,6 +43,8 @@ public class NodeDataWriter implements DescriptorListener {

private DocumentStore documentStore;

+ private long now; + private SortedMap<String, NodeStatus> knownNodes = new TreeMap<String, NodeStatus>();

@@ -60,11 +62,13 @@ public class NodeDataWriter implements DescriptorListener {

public NodeDataWriter(DescriptorSource descriptorSource, ReverseDomainNameResolver reverseDomainNameResolver, - LookupService lookupService, DocumentStore documentStore) { + LookupService lookupService, DocumentStore documentStore, + Time time) { this.descriptorSource = descriptorSource; this.reverseDomainNameResolver = reverseDomainNameResolver; this.lookupService = lookupService; this.documentStore = documentStore; + this.now = time.currentTimeMillis(); this.registerDescriptorListeners(); }

@@ -410,8 +414,6 @@ public class NodeDataWriter implements DescriptorListener { } }

- private long now = System.currentTimeMillis(); - private Map<String, Set<ExitListEntry>> exitListEntries = new HashMap<String, Set<ExitListEntry>>();

diff --git a/src/org/torproject/onionoo/ResourceServlet.java b/src/org/torproject/onionoo/ResourceServlet.java index 0bdb928..2869d5e 100644 --- a/src/org/torproject/onionoo/ResourceServlet.java +++ b/src/org/torproject/onionoo/ResourceServlet.java @@ -24,21 +24,24 @@ public class ResourceServlet extends HttpServlet {

private boolean maintenanceMode = false;

+ /* Called by servlet container, not by test class. */ public void init(ServletConfig config) throws ServletException { super.init(config); boolean maintenanceMode = config.getInitParameter("maintenance") != null && config.getInitParameter("maintenance").equals("1"); File outDir = new File(config.getInitParameter("outDir")); - this.init(maintenanceMode, outDir, true); + this.init(maintenanceMode, outDir, new Time()); }

+ /* Called (indirectly) by servlet container and (directly) by test + * class. */ protected void init(boolean maintenanceMode, File outDir, - boolean checkSummaryStale) { + Time time) { this.maintenanceMode = maintenanceMode; if (!maintenanceMode) { - ResponseBuilder.initialize(new DocumentStore(outDir), - checkSummaryStale); + ResponseBuilder.initialize(new DocumentStore(outDir, time), + time); } }

diff --git a/src/org/torproject/onionoo/ResponseBuilder.java b/src/org/torproject/onionoo/ResponseBuilder.java index 3c1898b..5cc1e04 100644 --- a/src/org/torproject/onionoo/ResponseBuilder.java +++ b/src/org/torproject/onionoo/ResponseBuilder.java @@ -21,7 +21,7 @@ public class ResponseBuilder {

private static long summaryFileLastModified = -1L; private static DocumentStore documentStore; - private static boolean checkSummaryStale = false; + private static Time time; private static boolean successfullyReadSummaryFile = false; private static String relaysPublishedString, bridgesPublishedString; private static List<String> relaysByConsensusWeight = null; @@ -36,9 +36,9 @@ public class ResponseBuilder { private static final long SUMMARY_MAX_AGE = 6L * 60L * 60L * 1000L;

public static void initialize(DocumentStore documentStoreParam, - boolean checkSummaryStaleParam) { + Time timeParam) { documentStore = documentStoreParam; - checkSummaryStale = checkSummaryStaleParam; + time = timeParam; readSummaryFile(); }

@@ -66,8 +66,8 @@ public class ResponseBuilder { successfullyReadSummaryFile = false; return; } - if (checkSummaryStale && newSummaryFileLastModified + SUMMARY_MAX_AGE - < System.currentTimeMillis()) { + if (newSummaryFileLastModified + SUMMARY_MAX_AGE + < time.currentTimeMillis()) { // TODO Does this actually solve anything? Should we instead // switch to a variant of the maintenance mode and re-check when // the next requests comes in that happens x seconds after this one? diff --git a/src/org/torproject/onionoo/ReverseDomainNameResolver.java b/src/org/torproject/onionoo/ReverseDomainNameResolver.java index bb10747..586da35 100644 --- a/src/org/torproject/onionoo/ReverseDomainNameResolver.java +++ b/src/org/torproject/onionoo/ReverseDomainNameResolver.java @@ -16,7 +16,7 @@ public class ReverseDomainNameResolver {

private class RdnsLookupWorker extends Thread { public void run() { - while (System.currentTimeMillis() - RDNS_LOOKUP_MAX_DURATION_MILLIS + while (time.currentTimeMillis() - RDNS_LOOKUP_MAX_DURATION_MILLIS <= startedRdnsLookups) { String rdnsLookupJob = null; synchronized (rdnsLookupJobs) { @@ -63,7 +63,7 @@ public class ReverseDomainNameResolver { this.address = address; } public void run() { - this.lookupStartedMillis = System.currentTimeMillis(); + this.lookupStartedMillis = time.currentTimeMillis(); try { String result = InetAddress.getByName(this.address).getHostName(); synchronized (this) { @@ -72,7 +72,7 @@ public class ReverseDomainNameResolver { } catch (UnknownHostException e) { /* We'll try again the next time. */ } - this.lookupCompletedMillis = System.currentTimeMillis(); + this.lookupCompletedMillis = time.currentTimeMillis(); this.parent.interrupt(); } public synchronized String getHostName() { @@ -83,6 +83,12 @@ public class ReverseDomainNameResolver { } }

+ private Time time; + + public ReverseDomainNameResolver(Time time) { + this.time = time; + } + private static final long RDNS_LOOKUP_MAX_REQUEST_MILLIS = 10L * 1000L; private static final long RDNS_LOOKUP_MAX_DURATION_MILLIS = 5L * 60L * 1000L; @@ -107,7 +113,7 @@ public class ReverseDomainNameResolver { }

public void startReverseDomainNameLookups() { - this.startedRdnsLookups = System.currentTimeMillis(); + this.startedRdnsLookups = this.time.currentTimeMillis(); this.rdnsLookupJobs = new HashSet<String>(); for (Map.Entry<String, Long> e : this.addressLastLookupTimes.entrySet()) { diff --git a/src/org/torproject/onionoo/Time.java b/src/org/torproject/onionoo/Time.java new file mode 100644 index 0000000..c969556 --- /dev/null +++ b/src/org/torproject/onionoo/Time.java @@ -0,0 +1,14 @@ +/* Copyright 2013 The Tor Project + * See LICENSE for licensing information */ +package org.torproject.onionoo; + +/* + * Wrapper for System.currentTimeMillis() that can be replaced with a + * custom time source for testing. + */ +public class Time { + public long currentTimeMillis() { + return System.currentTimeMillis(); + } +} + diff --git a/src/org/torproject/onionoo/WeightsDataWriter.java b/src/org/torproject/onionoo/WeightsDataWriter.java index b8e24ca..d95e19c 100644 --- a/src/org/torproject/onionoo/WeightsDataWriter.java +++ b/src/org/torproject/onionoo/WeightsDataWriter.java @@ -31,10 +31,13 @@ public class WeightsDataWriter implements DescriptorListener {

private DocumentStore documentStore;

+ private long now; + public WeightsDataWriter(DescriptorSource descriptorSource, - DocumentStore documentStore) { + DocumentStore documentStore, Time time) { this.descriptorSource = descriptorSource; this.documentStore = documentStore; + this.now = time.currentTimeMillis(); this.registerDescriptorListeners(); }

@@ -350,7 +353,6 @@ public class WeightsDataWriter implements DescriptorListener { return history; }

- private long now = System.currentTimeMillis(); private SortedMap<long[], double[]> compressHistory( SortedMap<long[], double[]> history) { SortedMap<long[], double[]> compressedHistory = diff --git a/test/org/torproject/onionoo/ResourceServletTest.java b/test/org/torproject/onionoo/ResourceServletTest.java index e61b12d..fbe7db7 100644 --- a/test/org/torproject/onionoo/ResourceServletTest.java +++ b/test/org/torproject/onionoo/ResourceServletTest.java @@ -43,6 +43,16 @@ public class ResourceServletTest { // 2013-04-24 12:22:22 private static long lastModified = 1366806142000L;

+ private static long currentTimeMillis = 1366806142000L; + + private class TestingTime extends Time { + public long currentTimeMillis() { + return currentTimeMillis; + } + } + + private Time testingTime = new TestingTime(); + private boolean maintenanceMode = false;

private class TestingHttpServletRequestWrapper @@ -181,7 +191,7 @@ public class ResourceServletTest {

private void makeRequest() throws IOException { ResourceServlet rs = new ResourceServlet(); - rs.init(maintenanceMode, this.tempOutDir, false); + rs.init(maintenanceMode, this.tempOutDir, this.testingTime); rs.doGet(this.request, this.response); }