commit b8386ce0e39aeadff82bdb9bc155ad6e69c23ec7 Author: iwakeh iwakeh@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)); + } + } + } + } + }