diff options
author | gjoranv <gv@verizonmedia.com> | 2019-03-09 00:33:21 +0100 |
---|---|---|
committer | gjoranv <gv@verizonmedia.com> | 2019-03-09 00:58:08 +0100 |
commit | c702a60877b640ef9ccf065515d7b9673221c38c (patch) | |
tree | 44dd096e6e1281dd98c19a06fe4dfa550bbeb660 /config-model | |
parent | 724878fe1108726593a822efd88ba967fee4a3ee (diff) |
Reimplement how to find all configs produced by a ConfigProducer.
- Search also super classes, not only implemented interfaces.
This is necessary to get the correct set of configs for Container
implementations and future ContainerCluster implementations.
Diffstat (limited to 'config-model')
3 files changed, 61 insertions, 40 deletions
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java b/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java index d954c69d144..c8b95d7497d 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java @@ -474,7 +474,7 @@ public final class VespaModel extends AbstractConfigProducerRoot implements Seri } private static Set<ConfigKey<?>> configsProduced(ConfigProducer cp) { - Set<ConfigKey<?>> ret = ReflectionUtil.configsProducedByInterface(cp.getClass(), cp.getConfigId()); + Set<ConfigKey<?>> ret = ReflectionUtil.getAllConfigsProduced(cp.getClass(), cp.getConfigId()); UserConfigRepo userConfigs = cp.getUserConfigs(); for (ConfigDefinitionKey userKey : userConfigs.configsProduced()) { ret.add(new ConfigKey<>(userKey.getName(), cp.getConfigId(), userKey.getNamespace())); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/utils/internal/ReflectionUtil.java b/config-model/src/main/java/com/yahoo/vespa/model/utils/internal/ReflectionUtil.java index 9913fde49b6..ef021872efb 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/utils/internal/ReflectionUtil.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/utils/internal/ReflectionUtil.java @@ -1,43 +1,36 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.utils.internal; +import com.google.common.reflect.TypeToken; import com.yahoo.config.ChangesRequiringRestart; import com.yahoo.config.ConfigInstance; import com.yahoo.vespa.config.ConfigKey; +import com.yahoo.vespa.model.ConfigProducer; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.util.LinkedHashSet; import java.util.Set; +import java.util.stream.Collectors; /** - * Utility class containing static methods for retrievinig information about the config producer tree. + * Utility class containing static methods for retrieving information about the config producer tree. * * @author Ulf Lilleengen * @author bjorncs - * @since 5.1 + * @author gjoranv */ public final class ReflectionUtil { private ReflectionUtil() { } - /** - * Returns a set of all the configs produced by a given producer. - * - * @param iface The config producer or interface to check for producers. - * @param configId The config id to use when creating keys. - * @return A set of config keys. - */ - public static Set<ConfigKey<?>> configsProducedByInterface(Class<?> iface, String configId) { - Set<ConfigKey<?>> ret = new LinkedHashSet<>(); - if (isConcreteProducer(iface)) { - ret.add(createConfigKeyFromInstance(iface.getEnclosingClass(), configId)); - } - for (Class<?> parentIface : iface.getInterfaces()) { - ret.addAll(configsProducedByInterface(parentIface, configId)); - } - return ret; + public static Set<ConfigKey<?>> getAllConfigsProduced(Class<? extends ConfigProducer> producerClass, String configId) { + // TypeToken is @Beta in guava, so consider implementing a simple recursive method instead. + TypeToken<? extends ConfigProducer>.TypeSet interfaces = TypeToken.of(producerClass).getTypes().interfaces(); + return interfaces.rawTypes().stream() + .filter(ReflectionUtil::isConcreteProducer) + .map(i -> createConfigKeyFromInstance(i.getEnclosingClass(), configId)) + .collect(Collectors.toSet()); } /** @@ -97,18 +90,26 @@ public final class ReflectionUtil { } } - private static boolean classIsConfigInstanceProducer(Class<?> clazz) { - return clazz.getName().equals(ConfigInstance.Producer.class.getName()); - } - private static boolean isConcreteProducer(Class<?> producerInterface) { + if (isRootConfigProducerInterface(producerInterface)) return false; + boolean parentIsConfigInstance = false; for (Class<?> ifaceParent : producerInterface.getInterfaces()) { - if (classIsConfigInstanceProducer(ifaceParent)) { + if (isConfigInstanceProducer(ifaceParent)) { parentIsConfigInstance = true; } } - return (ConfigInstance.Producer.class.isAssignableFrom(producerInterface) && parentIsConfigInstance && !classIsConfigInstanceProducer(producerInterface)); + return (ConfigInstance.Producer.class.isAssignableFrom(producerInterface) + && parentIsConfigInstance + && !isConfigInstanceProducer(producerInterface)); + } + + private static boolean isConfigInstanceProducer(Class<?> clazz) { + return clazz.getName().equals(ConfigInstance.Producer.class.getName()); + } + + private static boolean isRootConfigProducerInterface(Class<?> clazz) { + return clazz.getCanonicalName().equals(ConfigProducer.class.getCanonicalName()); } } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/utils/internal/ReflectionUtilTest.java b/config-model/src/test/java/com/yahoo/vespa/model/utils/internal/ReflectionUtilTest.java index 2825605c1f6..9032a8b05b7 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/utils/internal/ReflectionUtilTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/utils/internal/ReflectionUtilTest.java @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.utils.internal; +import com.yahoo.config.model.producer.AbstractConfigProducer; import com.yahoo.test.ArraytypesConfig; import com.yahoo.config.ChangesRequiringRestart; import com.yahoo.config.ConfigInstance; @@ -10,6 +11,7 @@ import org.junit.Test; import java.util.Set; +import static com.yahoo.vespa.model.utils.internal.ReflectionUtil.getAllConfigsProduced; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -19,26 +21,37 @@ import static org.junit.Assert.assertTrue; /** * @author Ulf Lilleengen * @author bjorncs + * @author gjoranv * @since 5.1 */ public class ReflectionUtilTest { - private static interface ComplexInterface extends SimpletypesConfig.Producer, ArraytypesConfig.Producer { - } + private static class SimpleProducer extends AbstractConfigProducer implements SimpletypesConfig.Producer { + SimpleProducer(AbstractConfigProducer parent, String subId) { super(parent, subId); } - private static class SimpleProducer implements SimpletypesConfig.Producer { @Override - public void getConfig(SimpletypesConfig.Builder builder) { - } + public void getConfig(SimpletypesConfig.Builder builder) { } } - private static class ComplexProducer implements ComplexInterface { + private interface ProducerInterface extends SimpletypesConfig.Producer, ArraytypesConfig.Producer { } + + private static class InterfaceImplementingProducer extends AbstractConfigProducer implements ProducerInterface { + InterfaceImplementingProducer(AbstractConfigProducer parent, String subId) { super(parent, subId); } + @Override - public void getConfig(ArraytypesConfig.Builder builder) { - } + public void getConfig(ArraytypesConfig.Builder builder) { } @Override - public void getConfig(SimpletypesConfig.Builder builder) { - } + public void getConfig(SimpletypesConfig.Builder builder) { } + } + + private static abstract class MyAbstractProducer extends AbstractConfigProducer implements SimpletypesConfig.Producer { + MyAbstractProducer(AbstractConfigProducer parent, String subId) { super(parent, subId); } + @Override + public void getConfig(SimpletypesConfig.Builder builder) { } + } + + private static class ConcreteProducer extends MyAbstractProducer { + ConcreteProducer(AbstractConfigProducer parent, String subId) { super(parent, subId); } } private static class RestartConfig extends ConfigInstance { @@ -56,16 +69,23 @@ public class ReflectionUtilTest { private static class NonRestartConfig extends ConfigInstance {} @Test - public void requireThatConfigsProducedByInterfaceTakesParentIntoAccount() { - Set<ConfigKey<?>> configs = ReflectionUtil.configsProducedByInterface(ComplexProducer.class, "foo"); + public void getAllConfigsProduced_includes_configs_produced_by_super_class() { + Set<ConfigKey<?>> configs = getAllConfigsProduced(ConcreteProducer.class, "foo"); + assertThat(configs.size(), is(1)); + assertTrue(configs.contains(new ConfigKey<>(SimpletypesConfig.CONFIG_DEF_NAME, "foo", SimpletypesConfig.CONFIG_DEF_NAMESPACE))); + } + + @Test + public void getAllConfigsProduced_includes_configs_produced_by_implemented_interface() { + Set<ConfigKey<?>> configs = getAllConfigsProduced(InterfaceImplementingProducer.class, "foo"); assertThat(configs.size(), is(2)); assertTrue(configs.contains(new ConfigKey<>(SimpletypesConfig.CONFIG_DEF_NAME, "foo", SimpletypesConfig.CONFIG_DEF_NAMESPACE))); assertTrue(configs.contains(new ConfigKey<>(ArraytypesConfig.CONFIG_DEF_NAME, "foo", ArraytypesConfig.CONFIG_DEF_NAMESPACE))); } @Test - public void requireThatConfigsProducedByInterfaceAreFound() { - Set<ConfigKey<?>> configs = ReflectionUtil.configsProducedByInterface(SimpleProducer.class, "foo"); + public void getAllConfigsProduced_includes_configs_directly_implemented_by_producer() { + Set<ConfigKey<?>> configs = getAllConfigsProduced(SimpleProducer.class, "foo"); assertThat(configs.size(), is(1)); assertTrue(configs.contains(new ConfigKey<>(SimpletypesConfig.CONFIG_DEF_NAME, "foo", SimpletypesConfig.CONFIG_DEF_NAMESPACE))); } |