Changes based on review comments

components/common/src/main/java/com/hotels/styx/configstore/ConfigStore.java

@@ -1,5 +1,5 @@
 /*
-  Copyright (C) 2013-2018 Expedia Inc.
+  Copyright (C) 2013-2019 Expedia Inc.
 
   Licensed under the Apache License, Version 2.0 (the "License");
   you may not use this file except in compliance with the License.
@@ -139,6 +139,21 @@ public <T> List<ConfigEntry<T>> startingWith(String rootKey, Class<T> type) {
                 .collect(toList());
     }
 
+    /**
+     * Gets the values associated with all entries under a given root key.
+     * For example, if the {@code rootKey} is "foo" then you would receive entries named "foo", "foo.bar", "foo.bar.baz", etc.
+     *
+     * @param rootKey root key
+     * @param type    type to cast values to
+     * @param <T>     type
+     * @return list of values
+     */
+    public <T> List<T> valuesStartingWith(String rootKey, Class<T> type) {
+        return startingWith(rootKey, type).stream()
+                .map(ConfigEntry::value)
+                .collect(toList());
+    }
+
     private <T> Observable<ConfigEntry<T>> watch(Predicate<String> keyMatcher, Class<T> type) {
         Observable<ConfigEntry<T>> subsequentStates = propagation
                 .filter(update -> keyMatcher.test(update.key()))

components/proxy/src/main/java/com/hotels/styx/Environment.java

@@ -17,7 +17,6 @@
 
 import com.google.common.eventbus.EventBus;
 import com.hotels.styx.api.MetricRegistry;
-import com.hotels.styx.api.configuration.Configuration;
 import com.hotels.styx.api.metrics.codahale.CodaHaleMetricRegistry;
 import com.hotels.styx.configstore.ConfigStore;
 import com.hotels.styx.proxy.HttpErrorStatusCauseLogger;
@@ -82,6 +81,12 @@ public MetricRegistry metricRegistry() {
         return serverEnvironment.metricRegistry();
     }
 
+    /**
+     * Use {@link #configuration()}.
+     *
+     * @return configuration
+     */
+    @Deprecated
     public StyxConfig styxConfig() {
         return configuration();
     }

components/proxy/src/main/java/com/hotels/styx/admin/handlers/PluginListHandler.java

@@ -34,7 +34,6 @@
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.Objects.requireNonNull;
 import static java.util.stream.Collectors.joining;
-import static java.util.stream.Collectors.toList;
 
 /**
  * Returns a simple HTML page with a list of plugins, split into enabled and disabled.
@@ -48,10 +47,7 @@ public PluginListHandler(ConfigStore configStore) {
 
     @Override
     public Eventual<LiveHttpResponse> handle(LiveHttpRequest request, HttpInterceptor.Context context) {
-        List<NamedPlugin> plugins = configStore.startingWith("plugins", NamedPlugin.class)
-                .stream()
-                .map(ConfigStore.ConfigEntry::value)
-                .collect(toList());
+        List<NamedPlugin> plugins = configStore.valuesStartingWith("plugins", NamedPlugin.class);
 
         Stream<NamedPlugin> enabled = plugins.stream().filter(NamedPlugin::enabled);
         Stream<NamedPlugin> disabled = plugins.stream().filter(plugin -> !plugin.enabled());

components/proxy/src/main/java/com/hotels/styx/startup/StyxServerComponents.java

@@ -57,7 +57,9 @@ private StyxServerComponents(Builder builder) {
         builder.loggingSetUp.setUp(environment);
 
         // TODO In further refactoring, we will probably want this loading to happen outside of this constructor call, so that it doesn't delay the admin server from starting up
-        this.plugins = loadPlugins(environment, builder.configuredPluginFactories);
+        this.plugins = builder.configuredPluginFactories == null
+                ? loadPlugins(environment)
+                : loadPlugins(environment, builder.configuredPluginFactories);
 
         this.services = mergeServices(
                 builder.servicesLoader.load(environment),

components/proxy/src/main/java/com/hotels/styx/startup/extensions/PluginLoadingForStartup.java

@@ -52,15 +52,14 @@ private PluginLoadingForStartup() {
      * @return plugins
      */
     public static List<NamedPlugin> loadPlugins(Environment environment, List<ConfiguredPluginFactory> factories) {
-        if (factories == null) {
-            List<ConfiguredPluginFactory> activePlugins = loadFactoriesFromConfig(environment);
-
-            return loadPluginsFromFactories(environment, activePlugins);
-        }
-
         return loadPluginsFromFactories(environment, factories);
     }
 
+    public static List<NamedPlugin> loadPlugins(Environment environment) {
+        List<ConfiguredPluginFactory> activePlugins = loadFactoriesFromConfig(environment);
+
+        return loadPluginsFromFactories(environment, activePlugins);
+    }
 
     private static List<ConfiguredPluginFactory> loadFactoriesFromConfig(Environment environment) {
         PluginFactoriesLoader loader = new PluginFactoriesLoader(PLUGIN_FACTORY_LOADING_FAILURE_HANDLING_STRATEGY);

components/proxy/src/test/java/com/hotels/styx/startup/extensions/PluginLoadingForStartupTest.java

@@ -69,7 +69,7 @@ public void suppliesConfiguredPlugin() {
                 "        testConfiguration: test-foo-bar\n";
 
 
-        List<NamedPlugin> plugins = PluginLoadingForStartup.loadPlugins(environment(yaml), null);
+        List<NamedPlugin> plugins = PluginLoadingForStartup.loadPlugins(environment(yaml));
 
         NamedPlugin plugin = plugins.get(0);
 
@@ -103,7 +103,7 @@ public void providesNamesForAllLoadedPlugins() {
                 "      config:\n" +
                 "        testConfiguration: instance3\n";
 
-        List<NamedPlugin> plugins = PluginLoadingForStartup.loadPlugins(environment(yaml), null);
+        List<NamedPlugin> plugins = PluginLoadingForStartup.loadPlugins(environment(yaml));
 
         List<String> pluginNames = plugins.stream()
                 .map(NamedPlugin::name)
@@ -125,7 +125,7 @@ public void throwsExceptionIfThereIsNoPluginMatchingActiveName() {
                 "      config:\n" +
                 "        testConfiguration: test-foo-bar\n";
 
-        PluginLoadingForStartup.loadPlugins(environment(yaml), null);
+        PluginLoadingForStartup.loadPlugins(environment(yaml));
     }
 
     @Test(expectedExceptions = RuntimeException.class)
@@ -141,7 +141,7 @@ public void throwsExceptionIfFactoryClassDoesNotExist() {
                 "    config:\n" +
                 "      testConfiguration: test-foo-bar\n";
 
-        PluginLoadingForStartup.loadPlugins(environment(yaml), null);
+        PluginLoadingForStartup.loadPlugins(environment(yaml));
     }
 
     @Test(expectedExceptions = RuntimeException.class, expectedExceptionsMessageRegExp = "(?s).*No active plugin specified.*")
@@ -156,7 +156,7 @@ public void throwsExceptionIfNoActivePluginSpecified() {
                 "      config:\n" +
                 "        testConfiguration: test-foo-bar\n";
 
-        PluginLoadingForStartup.loadPlugins(environment(yaml), null);
+        PluginLoadingForStartup.loadPlugins(environment(yaml));
     }
 
     @Test(expectedExceptions = RuntimeException.class, expectedExceptionsMessageRegExp = "(?s).*No list of all plugins specified.*")
@@ -165,7 +165,7 @@ public void throwsExceptionIfListOfPluginsNotSpecified() {
                 "plugins:\n" +
                 "  active: myPlugin\n";
 
-        PluginLoadingForStartup.loadPlugins(environment(yaml), null);
+        PluginLoadingForStartup.loadPlugins(environment(yaml));
     }
 
     @Test(expectedExceptions = RuntimeException.class, expectedExceptionsMessageRegExp =
@@ -180,7 +180,7 @@ public void throwsExceptionIfFactoryFailsToLoadPlugin() {
                 "        class: com.hotels.styx.startup.extensions.PluginLoadingForStartupTest$FailingPluginFactory\n" +
                 "        classPath: " + FIXTURES_CLASS_PATH + "\n";
 
-        PluginLoadingForStartup.loadPlugins(environment(yaml), null);
+        PluginLoadingForStartup.loadPlugins(environment(yaml));
     }
 
     @Test(expectedExceptions = RuntimeException.class, expectedExceptionsMessageRegExp =
@@ -209,7 +209,7 @@ public void attemptsToLoadAllPluginsEvenIfSomePluginFactoriesCannotBeLoaded() {
                     "        class: BadClassName\n" +
                     "        classPath: " + FIXTURES_CLASS_PATH + "\n";
 
-            PluginLoadingForStartup.loadPlugins(environment(yaml), null);
+            PluginLoadingForStartup.loadPlugins(environment(yaml));
         } catch (RuntimeException e) {
             assertThat(log.log(), hasItem(loggingEvent(ERROR, "Could not load plugin: pluginName=myPlugin1; factoryClass=.*", ConfigurationException.class, "Could not load a plugin factory for.*")));
             throw e;
@@ -244,7 +244,7 @@ public void attemptsToLoadAllPluginsEvenIfSomePluginFactoriesFailDuringExecution
                     "        class: com.hotels.styx.startup.extensions.PluginLoadingForStartupTest$FailingPluginFactory\n" +
                     "        classPath: " + FIXTURES_CLASS_PATH + "\n";
 
-            PluginLoadingForStartup.loadPlugins(environment(yaml), null);
+            PluginLoadingForStartup.loadPlugins(environment(yaml));
         } catch (RuntimeException e) {
             assertThat(log.log(), hasItem(loggingEvent(ERROR, "Could not load plugin: pluginName=myPlugin1; factoryClass=.*", RuntimeException.class, "plugin factory error")));
             throw e;
@@ -273,7 +273,7 @@ public void appliesDefaultMetricsScopeForPlugins() {
                 "        testConfiguration: test-foo-bar\n";
 
 
-        PluginLoadingForStartup.loadPlugins(environment(yaml), null);
+        PluginLoadingForStartup.loadPlugins(environment(yaml));
 
         assertThat(styxMetricsRegistry.counter("styx.plugins.myPlugin.initialised").getCount(), is(1L));
         assertThat(styxMetricsRegistry.counter("styx.plugins.myAnotherPlugin.initialised").getCount(), is(1L));