[onionoo/master] Remove circular dependency. - tor-commits

25 Jul 2014

commit a996e5cba4206c815fbfd4821f2d799885e66cc8
Author: Karsten Loesing karsten.loesing@gmx.net
Date:   Thu Jul 24 15:17:39 2014 +0200
Remove circular dependency.
It's okay that DocumentStore depends on UptimeStatus, but it's wrong that
    UptimeStatus also depends on DocumentStore.
---
 .../org/torproject/onionoo/docs/UptimeStatus.java  |   37 ++------
 .../onionoo/updater/UptimeStatusUpdater.java       |   23 ++++-
 .../org/torproject/onionoo/UptimeStatusTest.java   |   88 ++++++++++++--------
 3 files changed, 81 insertions(+), 67 deletions(-)

diff --git a/src/main/java/org/torproject/onionoo/docs/UptimeStatus.java b/src/main/java/org/torproject/onionoo/docs/UptimeStatus.java
index 1da11f0..c712172 100644
--- a/src/main/java/org/torproject/onionoo/docs/UptimeStatus.java
+++ b/src/main/java/org/torproject/onionoo/docs/UptimeStatus.java
@@ -6,14 +6,17 @@ import java.util.Scanner;
 import java.util.SortedSet;
 import java.util.TreeSet;
-import org.torproject.onionoo.util.ApplicationFactory;
 import org.torproject.onionoo.util.DateTimeHelper;
public class UptimeStatus extends Document {
-  private transient String fingerprint;
-
   private transient boolean isDirty = false;
+  public boolean isDirty() {
+    return this.isDirty;
+  }
+  public void clearDirty() {
+    this.isDirty = false;
+  }
private SortedSet<UptimeHistory> relayHistory =
       new TreeSet<UptimeHistory>();
@@ -33,19 +36,6 @@ public class UptimeStatus extends Document {
     return this.bridgeHistory;
   }
-  public static UptimeStatus loadOrCreate(String fingerprint) {
-    UptimeStatus uptimeStatus = (fingerprint == null) ?
-        ApplicationFactory.getDocumentStore().retrieve(
-            UptimeStatus.class, true) :
-        ApplicationFactory.getDocumentStore().retrieve(
-            UptimeStatus.class, true, fingerprint);
-    if (uptimeStatus == null) {
-      uptimeStatus = new UptimeStatus();
-    }
-    uptimeStatus.fingerprint = fingerprint;
-    return uptimeStatus;
-  }
-
   public void fromDocumentString(String documentString) {
     Scanner s = new Scanner(documentString);
     while (s.hasNextLine()) {
@@ -91,18 +81,9 @@ public class UptimeStatus extends Document {
     }
   }
-  public void storeIfChanged() {
-    if (this.isDirty) {
-      this.compressHistory(this.relayHistory);
-      this.compressHistory(this.bridgeHistory);
-      if (fingerprint == null) {
-        ApplicationFactory.getDocumentStore().store(this);
-      } else {
-        ApplicationFactory.getDocumentStore().store(this,
-            this.fingerprint);
-      }
-      this.isDirty = false;
-    }
+  public void compressHistory() {
+    this.compressHistory(this.relayHistory);
+    this.compressHistory(this.bridgeHistory);
   }
private void compressHistory(SortedSet<UptimeHistory> history) {
diff --git a/src/main/java/org/torproject/onionoo/updater/UptimeStatusUpdater.java b/src/main/java/org/torproject/onionoo/updater/UptimeStatusUpdater.java
index dd71e74..2a12caf 100644
--- a/src/main/java/org/torproject/onionoo/updater/UptimeStatusUpdater.java
+++ b/src/main/java/org/torproject/onionoo/updater/UptimeStatusUpdater.java
@@ -12,6 +12,7 @@ import org.torproject.descriptor.BridgeNetworkStatus;
 import org.torproject.descriptor.Descriptor;
 import org.torproject.descriptor.NetworkStatusEntry;
 import org.torproject.descriptor.RelayNetworkStatusConsensus;
+import org.torproject.onionoo.docs.DocumentStore;
 import org.torproject.onionoo.docs.UptimeStatus;
 import org.torproject.onionoo.util.ApplicationFactory;
 import org.torproject.onionoo.util.DateTimeHelper;
@@ -22,8 +23,11 @@ public class UptimeStatusUpdater implements DescriptorListener,
private DescriptorSource descriptorSource;
+  private DocumentStore documentStore;
+
   public UptimeStatusUpdater() {
     this.descriptorSource = ApplicationFactory.getDescriptorSource();
+    this.documentStore = ApplicationFactory.getDocumentStore();
     this.registerDescriptorListeners();
   }
@@ -108,9 +112,24 @@ public class UptimeStatusUpdater implements DescriptorListener,
private void updateStatus(boolean relay, String fingerprint,
       SortedSet<Long> newUptimeHours) {
-    UptimeStatus uptimeStatus = UptimeStatus.loadOrCreate(fingerprint);
+    UptimeStatus uptimeStatus = (fingerprint == null) ?
+        ApplicationFactory.getDocumentStore().retrieve(
+            UptimeStatus.class, true) :
+        ApplicationFactory.getDocumentStore().retrieve(
+            UptimeStatus.class, true, fingerprint);
+    if (uptimeStatus == null) {
+      uptimeStatus = new UptimeStatus();
+    }
     uptimeStatus.addToHistory(relay, newUptimeHours);
-    uptimeStatus.storeIfChanged();
+    if (uptimeStatus.isDirty()) {
+      uptimeStatus.compressHistory();
+      if (fingerprint == null) {
+        this.documentStore.store(uptimeStatus);
+      } else {
+        this.documentStore.store(uptimeStatus, fingerprint);
+      }
+      uptimeStatus.clearDirty();
+    }
   }
public String getStatsString() {
diff --git a/src/test/java/org/torproject/onionoo/UptimeStatusTest.java b/src/test/java/org/torproject/onionoo/UptimeStatusTest.java
index 671ffa3..9e235d5 100644
--- a/src/test/java/org/torproject/onionoo/UptimeStatusTest.java
+++ b/src/test/java/org/torproject/onionoo/UptimeStatusTest.java
@@ -3,6 +3,9 @@
 package org.torproject.onionoo;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
import java.util.Arrays;
 import java.util.TreeSet;
@@ -29,23 +32,25 @@ public class UptimeStatusTest {
@Test()
   public void testEmptyStatusNoWriteToDisk() {
-    UptimeStatus uptimeStatus = UptimeStatus.loadOrCreate(
-        MORIA1_FINGERPRINT);
-    uptimeStatus.storeIfChanged();
+    UptimeStatus uptimeStatus = this.documentStore.retrieve(
+        UptimeStatus.class, true, MORIA1_FINGERPRINT);
     assertEquals("Should make one retrieve attempt.", 1,
         this.documentStore.getPerformedRetrieveOperations());
-    assertEquals("Newly created uptime status with empty history should "
-        + "not be written to disk.", 0,
-        this.documentStore.getPerformedStoreOperations());
+    assertNull("Uptime status should not exist.", uptimeStatus);
+    uptimeStatus = new UptimeStatus();
+    assertFalse("Newly created uptime status should not be dirty.",
+        uptimeStatus.isDirty());
   }
@Test()
   public void testSingleHourWriteToDisk() {
-    UptimeStatus uptimeStatus = UptimeStatus.loadOrCreate(
-        MORIA1_FINGERPRINT);
+    UptimeStatus uptimeStatus = new UptimeStatus();
     uptimeStatus.addToHistory(true, new TreeSet<Long>(Arrays.asList(
         new Long[] { DateTimeHelper.parse("2013-12-20 00:00:00") })));
-    uptimeStatus.storeIfChanged();
+    uptimeStatus.compressHistory();
+    assertTrue("Changed uptime status should say it's dirty.",
+        uptimeStatus.isDirty());
+    this.documentStore.store(uptimeStatus, MORIA1_FINGERPRINT);
     assertEquals("History must contain single entry.", 1,
         uptimeStatus.getRelayHistory().size());
     UptimeHistory newUptimeHistory =
@@ -64,12 +69,12 @@ public class UptimeStatusTest {
@Test()
   public void testTwoConsecutiveHours() {
-    UptimeStatus uptimeStatus = UptimeStatus.loadOrCreate(
-        MORIA1_FINGERPRINT);
+    UptimeStatus uptimeStatus = new UptimeStatus();
     uptimeStatus.addToHistory(true, new TreeSet<Long>(Arrays.asList(
         new Long[] { DateTimeHelper.parse("2013-12-20 00:00:00"),
         DateTimeHelper.parse("2013-12-20 01:00:00") })));
-    uptimeStatus.storeIfChanged();
+    uptimeStatus.compressHistory();
+    this.documentStore.store(uptimeStatus, MORIA1_FINGERPRINT);
     assertEquals("History must contain single entry.", 1,
         uptimeStatus.getRelayHistory().size());
     UptimeHistory newUptimeHistory =
@@ -100,14 +105,15 @@ public class UptimeStatusTest {
   @Test()
   public void testGabelmooFillInGaps() {
     this.addGabelmooUptimeSample();
-    UptimeStatus uptimeStatus = UptimeStatus.loadOrCreate(
-        GABELMOO_FINGERPRINT);
+    UptimeStatus uptimeStatus = this.documentStore.retrieve(
+        UptimeStatus.class, true, GABELMOO_FINGERPRINT);
     uptimeStatus.addToHistory(true, new TreeSet<Long>(Arrays.asList(
         new Long[] { DateTimeHelper.parse("2013-09-09 02:00:00"),
         DateTimeHelper.parse("2013-12-20 00:00:00") })));
     assertEquals("Uncompressed history must contain five entries.", 5,
         uptimeStatus.getRelayHistory().size());
-    uptimeStatus.storeIfChanged();
+    uptimeStatus.compressHistory();
+    this.documentStore.store(uptimeStatus, GABELMOO_FINGERPRINT);
     assertEquals("Compressed history must contain one entry.", 1,
         uptimeStatus.getRelayHistory().size());
     UptimeHistory newUptimeHistory =
@@ -124,36 +130,39 @@ public class UptimeStatusTest {
   @Test()
   public void testAddExistingHourToIntervalStart() {
     this.addGabelmooUptimeSample();
-    UptimeStatus uptimeStatus = UptimeStatus.loadOrCreate(
-        GABELMOO_FINGERPRINT);
+    UptimeStatus uptimeStatus = this.documentStore.retrieve(
+        UptimeStatus.class, true, GABELMOO_FINGERPRINT);
     uptimeStatus.addToHistory(true, new TreeSet<Long>(Arrays.asList(
         new Long[] { DateTimeHelper.parse("2013-07-22 17:00:00") })));
-    uptimeStatus.storeIfChanged();
-    assertEquals("Unchanged history should not be written to disk.", 0,
-        this.documentStore.getPerformedStoreOperations());
+    uptimeStatus.compressHistory();
+    this.documentStore.store(uptimeStatus, GABELMOO_FINGERPRINT);
+    assertFalse("Unchanged history should not make uptime status dirty.",
+        uptimeStatus.isDirty());
   }
@Test()
   public void testAddExistingHourToIntervalEnd() {
     this.addGabelmooUptimeSample();
-    UptimeStatus uptimeStatus = UptimeStatus.loadOrCreate(
-        GABELMOO_FINGERPRINT);
+    UptimeStatus uptimeStatus = this.documentStore.retrieve(
+        UptimeStatus.class, true, GABELMOO_FINGERPRINT);
     uptimeStatus.addToHistory(true, new TreeSet<Long>(Arrays.asList(
         new Long[] { DateTimeHelper.parse("2013-09-09 01:00:00") })));
-    uptimeStatus.storeIfChanged();
-    assertEquals("Unchanged history should not be written to disk.", 0,
-        this.documentStore.getPerformedStoreOperations());
+    uptimeStatus.compressHistory();
+    this.documentStore.store(uptimeStatus, GABELMOO_FINGERPRINT);
+    assertFalse("Unchanged history should not make uptime status dirty.",
+        uptimeStatus.isDirty());
   }
@Test()
   public void testTwoHoursOverlappingWithIntervalStart() {
     this.addGabelmooUptimeSample();
-    UptimeStatus uptimeStatus = UptimeStatus.loadOrCreate(
-        GABELMOO_FINGERPRINT);
+    UptimeStatus uptimeStatus = this.documentStore.retrieve(
+        UptimeStatus.class, true, GABELMOO_FINGERPRINT);
     uptimeStatus.addToHistory(true, new TreeSet<Long>(Arrays.asList(
         new Long[] { DateTimeHelper.parse("2013-07-22 16:00:00"),
         DateTimeHelper.parse("2013-07-22 17:00:00")})));
-    uptimeStatus.storeIfChanged();
+    uptimeStatus.compressHistory();
+    this.documentStore.store(uptimeStatus, GABELMOO_FINGERPRINT);
     assertEquals("Compressed history must still contain three entries.",
         3, uptimeStatus.getRelayHistory().size());
     UptimeHistory newUptimeHistory =
@@ -170,12 +179,13 @@ public class UptimeStatusTest {
   @Test()
   public void testTwoHoursOverlappingWithIntervalEnd() {
     this.addGabelmooUptimeSample();
-    UptimeStatus uptimeStatus = UptimeStatus.loadOrCreate(
-        GABELMOO_FINGERPRINT);
+    UptimeStatus uptimeStatus = this.documentStore.retrieve(
+        UptimeStatus.class, true, GABELMOO_FINGERPRINT);
     uptimeStatus.addToHistory(true, new TreeSet<Long>(Arrays.asList(
         new Long[] { DateTimeHelper.parse("2013-09-09 01:00:00"),
         DateTimeHelper.parse("2013-09-09 02:00:00")})));
-    uptimeStatus.storeIfChanged();
+    uptimeStatus.compressHistory();
+    this.documentStore.store(uptimeStatus, GABELMOO_FINGERPRINT);
     assertEquals("Compressed history must now contain two entries.",
         2, uptimeStatus.getRelayHistory().size());
     UptimeHistory newUptimeHistory =
@@ -205,12 +215,14 @@ public class UptimeStatusTest {
   @Test()
   public void testAddRelayUptimeHours() {
     this.addAllRelaysAndBridgesUptimeSample();
-    UptimeStatus uptimeStatus = UptimeStatus.loadOrCreate(
-        ALL_RELAYS_AND_BRIDGES_FINGERPRINT);
+    UptimeStatus uptimeStatus = this.documentStore.retrieve(
+        UptimeStatus.class, true, ALL_RELAYS_AND_BRIDGES_FINGERPRINT);
     uptimeStatus.addToHistory(true, new TreeSet<Long>(Arrays.asList(
         new Long[] { DateTimeHelper.parse("2013-07-22 16:00:00"),
         DateTimeHelper.parse("2014-03-21 20:00:00")})));
-    uptimeStatus.storeIfChanged();
+    uptimeStatus.compressHistory();
+    this.documentStore.store(uptimeStatus,
+        ALL_RELAYS_AND_BRIDGES_FINGERPRINT);
     assertEquals("Compressed relay history must still contain one entry.",
         1, uptimeStatus.getRelayHistory().size());
     UptimeHistory newUptimeHistory =
@@ -227,12 +239,14 @@ public class UptimeStatusTest {
   @Test()
   public void testAddBridgeUptimeHours() {
     this.addAllRelaysAndBridgesUptimeSample();
-    UptimeStatus uptimeStatus = UptimeStatus.loadOrCreate(
-        ALL_RELAYS_AND_BRIDGES_FINGERPRINT);
+    UptimeStatus uptimeStatus = this.documentStore.retrieve(
+        UptimeStatus.class, true, ALL_RELAYS_AND_BRIDGES_FINGERPRINT);
     uptimeStatus.addToHistory(false, new TreeSet<Long>(Arrays.asList(
         new Long[] { DateTimeHelper.parse("2013-07-22 16:00:00"),
         DateTimeHelper.parse("2014-03-21 20:00:00")})));
-    uptimeStatus.storeIfChanged();
+    uptimeStatus.compressHistory();
+    this.documentStore.store(uptimeStatus,
+        ALL_RELAYS_AND_BRIDGES_FINGERPRINT);
     assertEquals("Compressed bridge history must still contain one "
         + "entry.", 1, uptimeStatus.getBridgeHistory().size());
     UptimeHistory newUptimeHistory =

    