Simplifying configuration and cleaning up instantiation

Author: akshaisarma

src/main/java/com/yahoo/bullet/BulletConfig.java

@@ -46,22 +46,15 @@ public class BulletConfig extends Config {
 
     public static final String RESULT_METADATA_METRICS_MAPPING = "bullet.result.metadata.metrics.mapping";
 
+    public static final String DEFAULT_CONFIGURATION_NAME = "bullet_defaults.yaml";
+
     /**
      * Constructor that loads specific file augmented with defaults.
      *
      * @param file YAML file to load.
      * @throws IOException if an error occurred with the file loading.
      */
     public BulletConfig(String file) throws IOException {
-        super(file);
-    }
-
-    /**
-     * Default constructor.
-     *
-     * @throws IOException if an error occurred with loading the default config.
-     */
-    public BulletConfig() throws IOException {
-        super();
+        super(file, DEFAULT_CONFIGURATION_NAME);
     }
 }

src/main/java/com/yahoo/bullet/Config.java

@@ -21,42 +21,33 @@
 import java.util.Set;
 
 @Slf4j
-public abstract class Config implements Serializable {
+public class Config implements Serializable {
     private Map<String, Object> data;
 
-    public static final String DEFAULT_CONFIGURATION_NAME = "bullet_defaults.yaml";
-    private String defaultConfiguration;
-
     /**
-     * Constructor that loads specific file augmented with defaults and the name of the default configuration file.
-     *
+     * Constructor that loads a specific file and loads the settings in that file.
      *
      * @param file YAML file to load.
-     * @param defaultConfigurationFile Default YAML file to load.
      * @throws IOException if an error occurred with the file loading.
      */
-    public Config(String file, String defaultConfigurationFile) throws IOException {
-        this.defaultConfiguration = defaultConfigurationFile;
-        data = loadConfigResource(file);
+    public Config(String file) throws IOException {
+        data = readYAML(file);
+        log.info("Configuration: {} ", data);
     }
 
     /**
-     * Constructor that loads specific file augmented with defaults.
+     * Constructor that loads specific file augmented with defaults and the name of the default configuration file.
      *
      * @param file YAML file to load.
+     * @param defaultConfigurationFile Default YAML file to load.
      * @throws IOException if an error occurred with the file loading.
      */
-    public Config(String file) throws IOException {
-        this(file, DEFAULT_CONFIGURATION_NAME);
-    }
-
-    /**
-     * Default constructor.
-     *
-     * @throws IOException if an error occurred with loading the default config.
-     */
-    public Config() throws IOException {
-        this(null);
+    public Config(String file, String defaultConfigurationFile) throws IOException {
+        this(defaultConfigurationFile);
+        // Override
+        Map<String, Object> specificConf = readYAML(file);
+        data.putAll(specificConf);
+        log.info("Final configuration: {} ", data);
     }
 
     /**
@@ -119,22 +110,24 @@ public void set(String key, Object value) {
         data.put(key, value);
     }
 
+    /**
+     * Merges another Config into this one.
+     *
+     * @param other The other {@link Config} to merge into this one.
+     */
+    public void merge(Config other) {
+        if (other != null) {
+            data.putAll(other.data);
+        }
+    }
+
     /**
      * Clears out the configuration.
      */
     public void clear() {
         data.clear();
     }
 
-    private Map<String, Object> loadConfigResource(String yamlFile) throws IOException {
-        Map<String, Object> defaultconf = readYAML(defaultConfiguration);
-        Map<String, Object> specificConf = readYAML(yamlFile);
-        // Override
-        defaultconf.putAll(specificConf);
-        log.info("Final configuration: {} ", defaultconf);
-        return defaultconf;
-    }
-
     /**
      * Reads a YAML file containing mappings and returns them as a {@link Map}.
      *

src/test/java/com/yahoo/bullet/BulletConfigTest.java

@@ -35,7 +35,7 @@ public void testCustomConfig() throws IOException {
 
     @Test
     public void testCustomProperties() throws IOException {
-        BulletConfig config = new BulletConfig();
+        BulletConfig config = new BulletConfig(null);
         Assert.assertNull(config.get("foo"));
         config.set("foo", "bar");
         Assert.assertEquals(config.get("foo"), "bar");
@@ -51,7 +51,7 @@ public void testGettingWithDefault() throws IOException {
 
     @Test
     public void testGettingMultipleProperties() throws IOException {
-        BulletConfig config = new BulletConfig();
+        BulletConfig config = new BulletConfig(null);
         config.clear();
         config.set("1", 1);
         config.set("pi", 3.14);
@@ -74,7 +74,7 @@ public void testGettingMultipleProperties() throws IOException {
 
     @Test
     public void testGettingMaskedProperties() throws IOException {
-        BulletConfig config = new BulletConfig();
+        BulletConfig config = new BulletConfig(null);
         config.clear();
         config.set("1", 1);
         config.set("pi", 3.14);
@@ -94,4 +94,28 @@ public void testGettingMaskedProperties() throws IOException {
         Assert.assertEquals(mappings.get("foo"), "bar");
         Assert.assertEquals(mappings.get("true"), true);
     }
+
+    @Test
+    public void testMerging() throws IOException {
+        BulletConfig config = new BulletConfig("src/test/resources/test_config.yaml");
+
+        int configSize = config.getAll(Optional.empty()).size();
+        Assert.assertEquals(config.get(BulletConfig.SPECIFICATION_MAX_DURATION), 10000L);
+        Assert.assertEquals(config.get(BulletConfig.AGGREGATION_MAX_SIZE), 100L);
+
+        Config another = new BulletConfig(null);
+        another.clear();
+        another.set(BulletConfig.SPECIFICATION_MAX_DURATION, 42L);
+        config.set("pi", 3.14);
+
+        config.merge(another);
+
+        // Test null
+        config.merge(null);
+
+        Assert.assertEquals(config.getAll(Optional.empty()).size(), configSize + 1);
+        Assert.assertEquals(config.get(BulletConfig.SPECIFICATION_MAX_DURATION), 42L);
+        Assert.assertEquals(config.get(BulletConfig.AGGREGATION_MAX_SIZE), 100L);
+        Assert.assertEquals(config.get("pi"), 3.14);
+    }
 }