[tor-commits] [onionoo/release] Fix comparison in writing uptime documents.

[karsten at torproject.org]

commit 203d5d885d4155ed719c5168430ba237bad734aa
Author: Karsten Loesing <karsten.loesing at gmx.net>
Date:   Mon Jan 15 11:11:20 2018 +0100

    Fix comparison in writing uptime documents.
    
    In the spec we write: "Contained graph history objects may contain
    null values if less than 20% of network statuses have been processed
    for a given time period."
    
    However, in the code we checked less than or equal. We should fix
    that.
---
 .../onionoo/writer/UptimeDocumentWriter.java       | 12 +++---
 .../onionoo/writer/UptimeDocumentWriterTest.java   | 44 +++++++++++++++++++++-
 2 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/src/main/java/org/torproject/onionoo/writer/UptimeDocumentWriter.java b/src/main/java/org/torproject/onionoo/writer/UptimeDocumentWriter.java
index 96537ef..f739b9e 100644
--- a/src/main/java/org/torproject/onionoo/writer/UptimeDocumentWriter.java
+++ b/src/main/java/org/torproject/onionoo/writer/UptimeDocumentWriter.java
@@ -225,8 +225,8 @@ public class UptimeDocumentWriter implements DocumentWriter {
       }
       while (hist.getStartMillis() >= intervalStartMillis
           + dataPointInterval) {
-        statusDataPoints.add(statusHours * 5 > dataPointIntervalHours
-            ? statusHours : -1);
+        statusDataPoints.add(statusHours * 5 < dataPointIntervalHours
+            ? -1 : statusHours);
         statusHours = -1;
         intervalStartMillis += dataPointInterval;
       }
@@ -238,8 +238,8 @@ public class UptimeDocumentWriter implements DocumentWriter {
             - Math.max(Math.max(hist.getStartMillis(),
             firstStatusStartMillis), intervalStartMillis))
             / DateTimeHelper.ONE_HOUR);
-        statusDataPoints.add(statusHours * 5 > dataPointIntervalHours
-            ? statusHours : -1);
+        statusDataPoints.add(statusHours * 5 < dataPointIntervalHours
+            ? -1 : statusHours);
         statusHours = -1;
         intervalStartMillis += dataPointInterval;
       }
@@ -251,8 +251,8 @@ public class UptimeDocumentWriter implements DocumentWriter {
           intervalStartMillis)) / DateTimeHelper.ONE_HOUR);
     }
     if (statusHours > 0) {
-      statusDataPoints.add(statusHours * 5 > dataPointIntervalHours
-          ? statusHours : -1);
+      statusDataPoints.add(statusHours * 5 < dataPointIntervalHours
+          ? -1 : statusHours);
     }
     List<Double> dataPoints = new ArrayList<>();
     for (int dataPointIndex = 0; dataPointIndex < statusDataPoints.size();
diff --git a/src/test/java/org/torproject/onionoo/writer/UptimeDocumentWriterTest.java b/src/test/java/org/torproject/onionoo/writer/UptimeDocumentWriterTest.java
index 3504509..b1ba2ed 100644
--- a/src/test/java/org/torproject/onionoo/writer/UptimeDocumentWriterTest.java
+++ b/src/test/java/org/torproject/onionoo/writer/UptimeDocumentWriterTest.java
@@ -77,6 +77,12 @@ public class UptimeDocumentWriterTest {
         (int) (FOUR_HOURS / ONE_SECOND), count, values);
   }
 
+  private void assertFiveYearGraph(UptimeDocument document, int graphs,
+      String first, String last, int count, List<Integer> values) {
+    this.assertGraph(document, graphs, "5_years", first, last,
+        (int) (DateTimeHelper.TEN_DAYS / ONE_SECOND), count, values);
+  }
+
   private void assertGraph(UptimeDocument document, int graphs,
       String graphName, String first, String last, int interval,
       int count, List<Integer> values) {
@@ -93,7 +99,7 @@ public class UptimeDocumentWriterTest {
         (int) history.getInterval());
     assertEquals("Factor should be 1.0 / 999.0.", 1.0 / 999.0,
         (double) history.getFactor(), 0.01);
-    assertEquals("There should be one data point per hour.", count,
+    assertEquals("There should be " + count + " data points.", count,
         (int) history.getCount());
     assertEquals("Count should be the same as the number of values.",
         count, history.getValues().size());
@@ -244,5 +250,41 @@ public class UptimeDocumentWriterTest {
         "2014-03-16 14:00:00", 2,
         Arrays.asList(new Integer[] { 499, 249 }));
   }
+
+  @Test
+  public void testFiveYearsLessThan20Percent() {
+    /* This relay was running for exactly 11 days and 23 hours over 2 years ago.
+     * This time period exactly matches 100% of a data point interval of 10 days
+     * plus a tiny bit less than 20% of the next data point interval. */
+    this.addStatusOneWeekSample("r 2012-03-05-00 287\n",
+        "r 2012-03-05-00 287\n");
+    UptimeDocumentWriter writer = new UptimeDocumentWriter();
+    DescriptorSourceFactory.getDescriptorSource().readDescriptors();
+    writer.writeDocuments();
+    assertEquals("Should write exactly one document.", 1,
+        this.documentStore.getPerformedStoreOperations());
+    UptimeDocument document = this.documentStore.getDocument(
+        UptimeDocument.class, GABELMOO_FINGERPRINT);
+    assertEquals("Should not contain any graph.", 0,
+        document.getUptime().size());
+  }
+
+  @Test
+  public void testFiveYearsAtLeast20Percent() {
+    /* This relay was running for exactly 12 days over 2 years ago. This time
+     * period exactly matches 100% of a data point interval of 10 days plus 20%
+     * of the next data point interval. */
+    this.addStatusOneWeekSample("r 2012-03-05-00 288\n",
+        "r 2012-03-05-00 288\n");
+    UptimeDocumentWriter writer = new UptimeDocumentWriter();
+    DescriptorSourceFactory.getDescriptorSource().readDescriptors();
+    writer.writeDocuments();
+    assertEquals("Should write exactly one document.", 1,
+        this.documentStore.getPerformedStoreOperations());
+    UptimeDocument document = this.documentStore.getDocument(
+        UptimeDocument.class, GABELMOO_FINGERPRINT);
+    this.assertFiveYearGraph(document, 1, "2012-03-10 00:00:00",
+        "2012-03-20 00:00:00", 2, Arrays.asList(new Integer[] { 999, 999 }));
+  }
 }