[tor-commits] [collector/master] Make sure the sync-markers are used allover; added some constants,

[karsten at torproject.org]

commit b8386ce0e39aeadff82bdb9bc155ad6e69c23ec7
Author: iwakeh <iwakeh at torproject.org>
Date:   Tue Oct 25 15:06:10 2016 +0200

    Make sure the sync-markers are used allover; added some constants,
    and a sync-marker test.
---
 .../torproject/collector/cron/CollecTorMain.java   |  6 +--
 .../collector/exitlists/ExitListDownloader.java    |  2 +-
 .../org/torproject/collector/sync/SyncManager.java |  4 +-
 .../collector/cron/CollecTorMainTest.java          | 59 ++++++++++++++++++++++
 4 files changed, 66 insertions(+), 5 deletions(-)

diff --git a/src/main/java/org/torproject/collector/cron/CollecTorMain.java b/src/main/java/org/torproject/collector/cron/CollecTorMain.java
index 4a3b6c6..c9c931f 100644
--- a/src/main/java/org/torproject/collector/cron/CollecTorMain.java
+++ b/src/main/java/org/torproject/collector/cron/CollecTorMain.java
@@ -31,7 +31,7 @@ public abstract class CollecTorMain extends SyncManager
       CollecTorMain.class);
 
   private static final long LIMIT_MB = 200;
-
+  public static final String SOURCES = "Sources";
   private final AtomicBoolean newConfigAvailable = new AtomicBoolean(false);
 
   protected Configuration config = new Configuration();
@@ -83,13 +83,13 @@ public abstract class CollecTorMain extends SyncManager
   }
 
   private boolean isSync() throws ConfigurationException {
-    String key = this.syncMarker() + "Sources";
+    String key = this.syncMarker() + SOURCES;
     return Key.has(key) && config.getSourceTypeSet(Key.valueOf(key))
         .contains(SourceType.Sync);
   }
 
   private boolean isSyncOnly() throws ConfigurationException {
-    String key = this.syncMarker() + "Sources";
+    String key = this.syncMarker() + SOURCES;
     return this.isSync() && config.getSourceTypeSet(Key.valueOf(key)).size() == 1;
   }
 
diff --git a/src/main/java/org/torproject/collector/exitlists/ExitListDownloader.java b/src/main/java/org/torproject/collector/exitlists/ExitListDownloader.java
index 81fab3d..3c2f403 100644
--- a/src/main/java/org/torproject/collector/exitlists/ExitListDownloader.java
+++ b/src/main/java/org/torproject/collector/exitlists/ExitListDownloader.java
@@ -58,7 +58,7 @@ public class ExitListDownloader extends CollecTorMain {
 
   @Override
   protected String syncMarker() {
-    return "Exitlists";
+    return "Exitlist";
   }
 
   @Override
diff --git a/src/main/java/org/torproject/collector/sync/SyncManager.java b/src/main/java/org/torproject/collector/sync/SyncManager.java
index d0a4407..1390d50 100644
--- a/src/main/java/org/torproject/collector/sync/SyncManager.java
+++ b/src/main/java/org/torproject/collector/sync/SyncManager.java
@@ -28,6 +28,8 @@ import java.util.Set;
 public class SyncManager {
 
   private static final Logger log = LoggerFactory.getLogger(SyncManager.class);
+  public static final String SYNCORIGINS = "SyncOrigins";
+
   private Date collectionDate;
 
   public SyncManager() { /* empty */ }
@@ -36,7 +38,7 @@ public class SyncManager {
   public void merge(Configuration conf, String marker,
       Map<String, Class<? extends Descriptor>> mapPathDesc)
       throws ConfigurationException {
-    URL[] sources = conf.getUrlArray(Key.valueOf(marker + "SyncOrigins"));
+    URL[] sources = conf.getUrlArray(Key.valueOf(marker + SYNCORIGINS));
     collectionDate = new Date();
     collectFromOtherInstances(sources, mapPathDesc.keySet(), marker, conf);
     mergeWithLocalStorage(sources, mapPathDesc, marker, conf);
diff --git a/src/test/java/org/torproject/collector/cron/CollecTorMainTest.java b/src/test/java/org/torproject/collector/cron/CollecTorMainTest.java
index befaad3..cfb3cf6 100644
--- a/src/test/java/org/torproject/collector/cron/CollecTorMainTest.java
+++ b/src/test/java/org/torproject/collector/cron/CollecTorMainTest.java
@@ -3,16 +3,28 @@
 
 package org.torproject.collector.cron;
 
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
+import org.torproject.collector.Main;
+import org.torproject.collector.conf.Configuration;
+import org.torproject.collector.conf.Key;
+import org.torproject.collector.sync.SyncManager;
+
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 
 import java.io.File;
 import java.io.IOException;
+import java.lang.reflect.Field;
 import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
 
 public class CollecTorMainTest {
 
@@ -33,5 +45,52 @@ public class CollecTorMainTest {
         "existant", "but", "no", "exception"));
   }
 
+  /* Verifies that every sync-marker is configured in the
+   * default collector.properties file and the other way around. */
+  @Test()
+  public void testSyncMarker() throws Exception {
+    Properties props = new Properties();
+    props.load(getClass().getClassLoader().getResourceAsStream(
+        Main.CONF_FILE));
+    String[] syncSettings = new String[] {CollecTorMain.SOURCES,
+        SyncManager.SYNCORIGINS};
+    Field ctmField = Main.class.getDeclaredField("collecTorMains");
+    ctmField.setAccessible(true);
+    Map<Key, Class<? extends CollecTorMain>> ctms
+        = (Map<Key, Class<? extends CollecTorMain>>) ctmField.get(Main.class);
+    List<String> markers = new ArrayList<>();
+    for (Map.Entry<Key, Class<? extends CollecTorMain>> entry
+        : ctms.entrySet()) {
+      String marker = entry.getValue().getConstructor(Configuration.class)
+          .newInstance(new Configuration()).syncMarker();
+      markers.add(marker);
+      String key = marker + CollecTorMain.SOURCES;
+      String sources = props.getProperty(key);
+      switch (marker) {
+        case "Relay":
+        case "Bridge":
+        case "Exitlist":
+          assertNotNull("Property '" + key
+              + "' not specified in " + Main.CONF_FILE + ".",
+              props.getProperty(key));
+          break;
+        default:
+          assertNull("Property '"  + key
+              + "' should not be listed in " + Main.CONF_FILE + ".",
+              props.getProperty(key));
+          break;
+      }
+    }
+    for (String propName : props.stringPropertyNames()) {
+      for (String part : syncSettings) {
+        if (propName.endsWith(part)) {
+          String key = propName.replace(part, "");
+          assertTrue("CollecTorMain '" + key + "' not specified in Main.class.",
+              markers.contains(key));
+        }
+      }
+    }
+  }
+
 }