Author: rmannibucau Date: Tue Jul 4 10:02:52 2017 New Revision: 1800744 URL: http://svn.apache.org/viewvc?rev=1800744&view=rev Log: trying to optimize a bit the CDI integration, can need some spec updates since it looks designed to be very lazy whereas it would be trivial to make it eager for most of it Modified: geronimo/components/config/trunk/impl/pom.xml geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigExtension.java geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigInjectionBean.java Modified: geronimo/components/config/trunk/impl/pom.xml URL: http://svn.apache.org/viewvc/geronimo/components/config/trunk/impl/pom.xml?rev=1800744&r1=1800743&r2=1800744&view=diff ============================================================================== --- geronimo/components/config/trunk/impl/pom.xml (original) +++ geronimo/components/config/trunk/impl/pom.xml Tue Jul 4 10:02:52 2017 @@ -103,6 +103,7 @@ 1 true + false Modified: geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigExtension.java URL: http://svn.apache.org/viewvc/geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigExtension.java?rev=1800744&r1=1800743&r2=1800744&view=diff ============================================================================== --- geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigExtension.java (original) +++ geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigExtension.java Tue Jul 4 10:02:52 2017 @@ -16,90 +16,383 @@ */ package org.apache.geronimo.config.cdi; -import java.lang.reflect.Type; -import java.util.ArrayList; -import java.util.HashSet; -import java.util.List; -import java.util.Optional; -import java.util.Set; -import java.util.stream.Collectors; +import org.apache.geronimo.config.ConfigImpl; +import org.eclipse.microprofile.config.Config; +import org.eclipse.microprofile.config.inject.ConfigProperty; +import org.eclipse.microprofile.config.spi.ConfigProviderResolver; +import javax.enterprise.context.spi.CreationalContext; import javax.enterprise.event.Observes; import javax.enterprise.inject.spi.AfterBeanDiscovery; import javax.enterprise.inject.spi.AfterDeploymentValidation; +import javax.enterprise.inject.spi.AnnotatedMember; +import javax.enterprise.inject.spi.AnnotatedType; import javax.enterprise.inject.spi.BeanManager; +import javax.enterprise.inject.spi.BeforeBeanDiscovery; import javax.enterprise.inject.spi.BeforeShutdown; -import javax.enterprise.inject.spi.DeploymentException; import javax.enterprise.inject.spi.Extension; import javax.enterprise.inject.spi.InjectionPoint; import javax.enterprise.inject.spi.ProcessInjectionPoint; import javax.inject.Provider; +import java.lang.reflect.ParameterizedType; +import java.lang.reflect.Type; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.function.Function; +import java.util.stream.Stream; -import org.apache.geronimo.config.DefaultConfigProvider; -import org.eclipse.microprofile.config.Config; -import org.eclipse.microprofile.config.ConfigProvider; -import org.eclipse.microprofile.config.inject.ConfigProperty; +import static java.util.function.Function.identity; +import static java.util.stream.Collectors.toMap; +import static org.eclipse.microprofile.config.ConfigProvider.getConfig; /** * @author Mark Struberg */ public class ConfigExtension implements Extension { + private static final Object NULL = new Object(); + private Config config; + private ConfigProviderResolver resolver; + + private Set injections = new HashSet<>(); + private List deploymentProblems = new ArrayList<>(); - private Set injectionPoints = new HashSet<>(); + void init(@Observes final BeforeBeanDiscovery beforeBeanDiscovery, final BeanManager bm) { + resolver = ConfigProviderResolver.instance(); + config = getConfig(); + } public void collectConfigProducer(@Observes ProcessInjectionPoint pip) { - ConfigProperty configProperty = pip.getInjectionPoint().getAnnotated().getAnnotation(ConfigProperty.class); + final InjectionPoint injectionPoint = pip.getInjectionPoint(); + final ConfigProperty configProperty = injectionPoint.getAnnotated().getAnnotation(ConfigProperty.class); if (configProperty != null) { - injectionPoints.add(pip.getInjectionPoint()); + Injection injection = new Injection(injectionPoint.getType()); + final String key = getConfigKey(injectionPoint, configProperty); + final boolean defaultUnset = isDefaultUnset(configProperty.defaultValue()); + if (!injections.add(injection)) { + final Injection ref = injection; + injection = injections.stream().filter(i -> i.equals(ref)).findFirst().get(); + } + injection.keys.add(key); + injection.defaultValues.add(configProperty.defaultValue()); + + final ConfigImpl configImpl = unwrapConfig(); + + // what about lazy runtime lookup, not consistent with tck and system prop usage, for now assume optional=optional ;) + boolean hasValue = true; + if (defaultUnset) { // value validation + if (ParameterizedType.class.isInstance(injection.type)) { + final ParameterizedType pt = ParameterizedType.class.cast(injection.type); + if (pt.getRawType() != Optional.class && !configImpl.getOptionalValue(key, String.class).isPresent()) { + hasValue = false; + } + } else if (!configImpl.getOptionalValue(key, String.class).isPresent()) { + hasValue = false; + } + if (!hasValue) { + deploymentProblems.add(new IllegalArgumentException("Missing converter for '" + key + "' from " + injectionPoint)); + } + } + + Class instanceType = null; + if (ParameterizedType.class.isInstance(injection.type)) { // converters validation + final ParameterizedType pt = ParameterizedType.class.cast(injection.type); + if (pt.getRawType() == Provider.class && pt.getActualTypeArguments().length == 1 && Class.class.isInstance(pt.getActualTypeArguments()[0]) + && !configImpl.getConverters().containsKey(Class.class.cast(pt.getActualTypeArguments()[0]))) { + instanceType = Class.class.cast(pt.getActualTypeArguments()[0]); + } // else if Optional it is fine, else we don't know how to process + } else if (Class.class.isInstance(injection.type)) { + instanceType = Class.class.cast(injection.type); + } + if (instanceType != null) { // validate we have a converter + we can convert the existing value + if (!configImpl.getConverters().containsKey(instanceType)) { + deploymentProblems.add(new IllegalArgumentException("Missing converter for '" + key + "' from " + injectionPoint)); + } else if (hasValue) { + try { + configImpl.getConverters().get(injection.type).convert(configImpl.getValue(key)); + } catch (final RuntimeException re) { + deploymentProblems.add(re); + } + } + } } } public void registerConfigProducer(@Observes AfterBeanDiscovery abd, BeanManager bm) { - Set types = injectionPoints.stream() - .filter(ip -> ip.getType() instanceof Class) - .map(ip -> (Class) ip.getType()) - .collect(Collectors.toSet()); - - // Provider and Optional are ParameterizedTypes and not a Class, so we need to add them manually - types.add(Provider.class); - types.add(Optional.class); - - types.forEach(type -> abd.addBean(new ConfigInjectionBean(bm, type))); + injections.stream() + .flatMap(injection -> { + final Function, String> keyProvider; + if (injection.keys.size() == 1) { + final String key = injection.keys.iterator().next(); + keyProvider = ctx -> key; + } else { + keyProvider = ctx -> getName(findInjectionPoint(bm, ctx)); + } + + if (ParameterizedType.class.isInstance(injection.type)) { + final ParameterizedType paramType = ParameterizedType.class.cast(injection.type); + final Type rawType = paramType.getRawType(); + + // todo: do we care of Instance injection? doesnt make much sense right? + if (Provider.class == rawType && paramType.getActualTypeArguments().length == 1) { + if (!Class.class.isInstance(paramType.getActualTypeArguments()[0])) { + deploymentProblems.add(new IllegalArgumentException("@ConfigProperty can only be used with Provider where T is a Class")); + return Stream.empty(); + } + final Class providerType = Class.class.cast(paramType.getActualTypeArguments()[0]); + return Stream.of(new ConfigInjectionBean>(injection.type) { + @Override + public Provider create(final CreationalContext> context) { + return () -> config.getValue(keyProvider.apply(context), providerType); + } + }); + } else if (Optional.class == rawType && paramType.getActualTypeArguments().length == 1) { + if (!Class.class.isInstance(paramType.getActualTypeArguments()[0])) { + deploymentProblems.add(new IllegalArgumentException("@ConfigProperty can only be used with Optional where T is a Class")); + return null; + } + final Class optionalType = Class.class.cast(paramType.getActualTypeArguments()[0]); + return Stream.of(new ConfigInjectionBean>(injection.type) { + @Override + public Optional create(final CreationalContext> context) { + return config.getOptionalValue(keyProvider.apply(context), optionalType); + } + }); + } else { + deploymentProblems.add(new IllegalArgumentException("Unsupported parameterized type " + paramType)); + return Stream.empty(); + } + } else if (Class.class.isInstance(injection.type)) { + final Class clazz = Class.class.cast(injection.type); + final ConfigInjectionBean bean; + if (injection.defaultValues.isEmpty()) { + bean = new ConfigInjectionBean(injection.type) { + @Override + public Object create(final CreationalContext context) { + return config.getOptionalValue(keyProvider.apply(context), clazz); + } + }; + } else if (injection.defaultValues.size() == 1) { // common enough to be optimized + final String defVal = injection.defaultValues.iterator().next(); + final Object alternativeVal = isDefaultUnset(defVal) ? null : unwrapConfig().convert(defVal, clazz); + bean = new ConfigInjectionBean(injection.type) { + @Override + public Object create(final CreationalContext context) { + final Optional optionalValue = config.getOptionalValue(keyProvider.apply(context), clazz); + return optionalValue.orElse(alternativeVal); + } + }; + } else { // sadly we need to get back to the injection point to know which one we need to use + final Map prepared = injection.defaultValues.stream() + .collect(toMap(identity(), k -> isDefaultUnset(k) ? NULL : unwrapConfig().convert(k, clazz), (a, b) -> b)); + bean = new ConfigInjectionBean(injection.type) { + @Override + public Object create(final CreationalContext context) { + final InjectionPoint ip = findInjectionPoint(bm, context); + if (ip == null) { + throw new IllegalStateException("Could not retrieve InjectionPoint"); + } + return config.getOptionalValue(ConfigExtension.this.getName(ip), clazz) + .orElseGet(() -> { + final Object val = prepared.get(ip.getAnnotated().getAnnotation(ConfigProperty.class).defaultValue()); + return val == NULL ? null : val; + }); + } + }; + } + + final Collection> beans = new ArrayList<>(); + beans.add(bean); + + // is adding these beans is that useful? we captured them all so only a programmatic lookup would justify it + // and not sure it would be done this way anyway + final ParameterizedTypeImpl providerType = new ParameterizedTypeImpl(Provider.class, injection.type); + if (injections.stream().noneMatch(i -> i.type.equals(providerType))) { + beans.add(new ConfigInjectionBean>(providerType) { + @Override + public Provider create(final CreationalContext> context) { + return () -> bean.create(context); + } + }); + } + + final ParameterizedTypeImpl optionalType = new ParameterizedTypeImpl(Optional.class, injection.type); + if (injections.stream().noneMatch(i -> i.type.equals(optionalType))) { + beans.add(new ConfigInjectionBean>(optionalType) { + @Override + public Optional create(final CreationalContext> context) { + return Optional.ofNullable(bean.create(context)); + } + }); + } + + return beans.stream(); + } else { + deploymentProblems.add(new IllegalArgumentException("Unknown type " + injection.type)); + return Stream.empty(); + } + }) + .forEach(abd::addBean); } public void validate(@Observes AfterDeploymentValidation add) { - List deploymentProblems = new ArrayList<>(); + deploymentProblems.forEach(add::addDeploymentProblem); + injections.clear(); + deploymentProblems.clear(); + } - config = ConfigProvider.getConfig(); + public void shutdown(@Observes BeforeShutdown bsd) { + resolver.releaseConfig(config); + } + + private ConfigImpl unwrapConfig() { + return ConfigImpl.class.cast(config); + } - for (InjectionPoint injectionPoint : injectionPoints) { - Type type = injectionPoint.getType(); - ConfigProperty configProperty = injectionPoint.getAnnotated().getAnnotation(ConfigProperty.class); - if (type instanceof Class) { - // a direct injection of a ConfigProperty - // that means a Converter must exist. - String key = ConfigInjectionBean.getConfigKey(injectionPoint, configProperty); - if ((isDefaultUnset(configProperty.defaultValue())) - && !config.getOptionalValue(key, (Class) type).isPresent()) { - deploymentProblems.add("No Config Value exists for " + key); + private String getName(final InjectionPoint ip) { + final ConfigProperty annotation = ip.getAnnotated().getAnnotation(ConfigProperty.class); + final String name = annotation.name(); + return isDefaultUnset(name) ? getConfigKey(ip, annotation) : name; + } + + /** + * Get the property key to use. + * In case the {@link ConfigProperty#name()} is empty we will try to determine the key name from the InjectionPoint. + */ + private static String getConfigKey(InjectionPoint ip, ConfigProperty configProperty) { + String key = configProperty.name(); + if (!key.isEmpty()) { + return key; + } + if (ip.getAnnotated() instanceof AnnotatedMember) { + AnnotatedMember member = (AnnotatedMember) ip.getAnnotated(); + AnnotatedType declaringType = member.getDeclaringType(); + if (declaringType != null) { + String[] parts = declaringType.getJavaClass().getName().split("."); + String cn = parts[parts.length - 1]; + parts[parts.length - 1] = Character.toLowerCase(cn.charAt(0)) + (cn.length() > 1 ? cn.substring(1) : ""); + StringBuilder sb = new StringBuilder(parts[0]); + for (int i = 1; i < parts.length; i++) { + sb.append(".").append(parts[i]); } + + // now add the field name + sb.append(".").append(member.getJavaMember().getName()); + return sb.toString(); } } - if (!deploymentProblems.isEmpty()) { - add.addDeploymentProblem(new DeploymentException("Error while validating Configuration\n" - + String.join("\n", deploymentProblems))); - } + throw new IllegalStateException("Could not find default name for @ConfigProperty InjectionPoint " + ip); + } + private static boolean isDefaultUnset(String defaultValue) { + return defaultValue == null || defaultValue.length() == 0 || defaultValue.equals(ConfigProperty.UNCONFIGURED_VALUE); } - public void shutdown(@Observes BeforeShutdown bsd) { - DefaultConfigProvider.instance().releaseConfig(config); + private static InjectionPoint findInjectionPoint(final BeanManager bm, final CreationalContext ctx) { + return InjectionPoint.class.cast( + bm.getReference(bm.resolve(bm.getBeans(InjectionPoint.class)), InjectionPoint.class, ctx)); } + private static final class Injection { + private final Type type; + private final Collection keys = new ArrayList<>(); + private final Collection defaultValues = new ArrayList<>(); - static boolean isDefaultUnset(String defaultValue) { - return defaultValue == null || defaultValue.length() == 0 || defaultValue.equals(ConfigProperty.UNCONFIGURED_VALUE); + private Injection(final Type type) { + this.type = type; + } + + @Override + public boolean equals(final Object o) { + if (this == o) { + return true; + } + if (o == null || Injection.class != o.getClass()) { + return false; + } + final Injection injection = Injection.class.cast(o); + return Objects.equals(type, injection.type); + } + + @Override + public int hashCode() { + return type.hashCode(); + } + } + + private class ParameterizedTypeImpl implements ParameterizedType { + private final Type rawType; + private final Type[] types; + + private ParameterizedTypeImpl(final Type raw, final Type... types) { + this.rawType = raw; + this.types = types; + } + + @Override + public Type[] getActualTypeArguments() { + return types.clone(); + } + + @Override + public Type getOwnerType() { + return null; + } + + @Override + public Type getRawType() { + return rawType; + } + + @Override + public int hashCode() { + return Arrays.hashCode(types) ^ rawType.hashCode(); + } + + @Override + public boolean equals(final Object obj) { + if (this == obj) { + return true; + } + if (ParameterizedType.class.isInstance(obj)) { + final ParameterizedType that = ParameterizedType.class.cast(obj); + final Type thatRawType = that.getRawType(); + return (rawType == null ? thatRawType == null : rawType.equals(thatRawType)) + && Arrays.equals(types, that.getActualTypeArguments()); + } + return false; + } + + @Override + public String toString() { + final StringBuilder buffer = new StringBuilder(); + buffer.append(Class.class.cast(rawType).getName()); + final Type[] actualTypes = getActualTypeArguments(); + if (actualTypes.length > 0) { + buffer.append("<"); + final int length = actualTypes.length; + for (int i = 0; i < length; i++) { + if (actualTypes[i] instanceof Class) { + buffer.append(((Class) actualTypes[i]).getSimpleName()); + } else { + buffer.append(actualTypes[i].toString()); + } + if (i != actualTypes.length - 1) { + buffer.append(","); + } + } + + buffer.append(">"); + } + return buffer.toString(); + } } } Modified: geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigInjectionBean.java URL: http://svn.apache.org/viewvc/geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigInjectionBean.java?rev=1800744&r1=1800743&r2=1800744&view=diff ============================================================================== --- geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigInjectionBean.java (original) +++ geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigInjectionBean.java Tue Jul 4 10:02:52 2017 @@ -16,59 +16,40 @@ */ package org.apache.geronimo.config.cdi; -import java.io.IOException; -import java.io.Serializable; -import java.lang.annotation.Annotation; -import java.lang.reflect.ParameterizedType; -import java.lang.reflect.Type; -import java.util.Collections; -import java.util.HashSet; -import java.util.Optional; -import java.util.Set; +import org.eclipse.microprofile.config.inject.ConfigProperty; import javax.enterprise.context.Dependent; import javax.enterprise.context.spi.CreationalContext; -import javax.enterprise.inject.spi.Annotated; -import javax.enterprise.inject.spi.AnnotatedMember; -import javax.enterprise.inject.spi.AnnotatedType; import javax.enterprise.inject.spi.Bean; -import javax.enterprise.inject.spi.BeanManager; import javax.enterprise.inject.spi.InjectionPoint; import javax.enterprise.inject.spi.PassivationCapable; import javax.enterprise.util.AnnotationLiteral; -import javax.inject.Provider; - -import org.apache.geronimo.config.ConfigImpl; -import org.eclipse.microprofile.config.Config; -import org.eclipse.microprofile.config.ConfigProvider; -import org.eclipse.microprofile.config.inject.ConfigProperty; -import org.eclipse.microprofile.config.spi.ConfigProviderResolver; +import java.lang.annotation.Annotation; +import java.lang.reflect.ParameterizedType; +import java.lang.reflect.Type; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; /** * @author Mark Struberg */ -public class ConfigInjectionBean implements Bean, PassivationCapable { +public abstract class ConfigInjectionBean implements Bean, PassivationCapable { private final static Set QUALIFIERS = new HashSet<>(); static { QUALIFIERS.add(new ConfigPropertyLiteral()); } - private final BeanManager bm; private final Class rawType; private final Set types; + private final String id; - /** - * only access via {@link #getConfig(} - */ - private Config _config; - - public ConfigInjectionBean(BeanManager bm, Type type) { - this.bm = bm; - - types = new HashSet<>(); - types.add(type); - rawType = getRawType(type); + ConfigInjectionBean(Type type) { + this.types = new HashSet<>(); + this.types.add(type); + this.rawType = getRawType(type); + this.id = "ConfigInjectionBean_" + type.toString(); } private Class getRawType(Type type) { @@ -86,7 +67,7 @@ public class ConfigInjectionBean impl @Override public Set getInjectionPoints() { - return Collections.EMPTY_SET; + return Collections.emptySet(); } @Override @@ -100,90 +81,8 @@ public class ConfigInjectionBean impl } @Override - public T create(CreationalContext context) { - Set> beans = bm.getBeans(InjectionPoint.class); - Bean bean = bm.resolve(beans); - InjectionPoint ip = (InjectionPoint) bm.getReference(bean, InjectionPoint.class, context); - if (ip == null) { - throw new IllegalStateException("Could not retrieve InjectionPoint"); - } - Annotated annotated = ip.getAnnotated(); - ConfigProperty configProperty = annotated.getAnnotation(ConfigProperty.class); - String key = configProperty.name(); - String defaultValue = configProperty.defaultValue(); - - if (annotated.getBaseType() instanceof ParameterizedType) { - ParameterizedType paramType = (ParameterizedType) annotated.getBaseType(); - Type rawType = paramType.getRawType(); - - // handle Provider - if (rawType instanceof Class && ((Class) rawType).isAssignableFrom(Provider.class) && paramType.getActualTypeArguments().length == 1) { - Class clazz = (Class) paramType.getActualTypeArguments()[0]; //X TODO check type again, etc - return (T) new ConfigValueProvider(getConfig(), key, clazz); - } - - // handle Optional - if (rawType instanceof Class && ((Class) rawType).isAssignableFrom(Optional.class) && paramType.getActualTypeArguments().length == 1) { - Class clazz = (Class) paramType.getActualTypeArguments()[0]; //X TODO check type again, etc - return (T) getConfig().getOptionalValue(key, clazz); - } - } - else { - Class clazz = (Class) annotated.getBaseType(); - if (ConfigExtension.isDefaultUnset(defaultValue)) { - return (T) getConfig().getValue(key, clazz); - } - else { - Config config = getConfig(); - return (T) config.getOptionalValue(key, clazz) - .orElse(((ConfigImpl) config).convert(defaultValue, clazz)); - } - } - - throw new IllegalStateException("unhandled ConfigProperty"); - } - - - /** - * Get the property key to use. - * In case the {@link ConfigProperty#name()} is empty we will try to determine the key name from the InjectionPoint. - */ - public static String getConfigKey(InjectionPoint ip, ConfigProperty configProperty) { - String key = configProperty.name(); - if (key.length() > 0) { - return key; - } - if (ip.getAnnotated() instanceof AnnotatedMember) { - AnnotatedMember member = (AnnotatedMember) ip.getAnnotated(); - AnnotatedType declaringType = member.getDeclaringType(); - if (declaringType != null) { - String[] parts = declaringType.getJavaClass().getName().split("."); - String cn = parts[parts.length-1]; - parts[parts.length-1] = Character.toLowerCase(cn.charAt(0)) + (cn.length() > 1 ? cn.substring(1) : ""); - StringBuilder sb = new StringBuilder(parts[0]); - for (int i = 1; i < parts.length; i++) { - sb.append(".").append(parts[i]); - } - - // now add the field name - sb.append(".").append(member.getJavaMember().getName()); - return sb.toString(); - } - } - - throw new IllegalStateException("Could not find default name for @ConfigProperty InjectionPoint " + ip); - } - - public Config getConfig() { - if (_config == null) { - _config = ConfigProvider.getConfig(); - } - return _config; - } - - @Override public void destroy(T instance, CreationalContext context) { - + // no-op } @Override @@ -208,7 +107,7 @@ public class ConfigInjectionBean impl @Override public Set> getStereotypes() { - return Collections.EMPTY_SET; + return Collections.emptySet(); } @Override @@ -218,7 +117,7 @@ public class ConfigInjectionBean impl @Override public String getId() { - return "ConfigInjectionBean_" + rawType.getName(); + return id; } private static class ConfigPropertyLiteral extends AnnotationLiteral implements ConfigProperty { @@ -232,27 +131,4 @@ public class ConfigInjectionBean impl return ""; } } - - public static class ConfigValueProvider implements Provider, Serializable { - private transient Config config; - private final String key; - private final Class type; - - ConfigValueProvider(Config config, String key, Class type) { - this.config = config; - this.key = key; - this.type = type; - } - - @Override - public T get() { - return (T) config.getValue(key, type); - } - - private void readObject(java.io.ObjectInputStream in) throws IOException, ClassNotFoundException { - in.defaultReadObject(); - config = ConfigProviderResolver.instance().getConfig(); - } - - } }