From 47c77cb16550dea2054908c7e7fecc21d357bacb Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Mon, 18 Jan 2016 17:41:55 +0000 Subject: [PATCH] Fix discovery client if enabled but no impl available If user has @EnableDiscoveryClient but no implementation (e.g. if he just has @EnableZuulProxy) the autoconfig will now kick in and provide an instance of NoopDiscoveryClient. Fixes gh-80 --- .../EnableDiscoveryClientImportSelector.java | 9 ++++- .../util/SpringFactoryImportSelector.java | 38 ++++++++++--------- 2 files changed, 28 insertions(+), 19 deletions(-) diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/EnableDiscoveryClientImportSelector.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/EnableDiscoveryClientImportSelector.java index 030b8b10..c04769f8 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/EnableDiscoveryClientImportSelector.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/EnableDiscoveryClientImportSelector.java @@ -25,8 +25,8 @@ import org.springframework.core.annotation.Order; * @author Spencer Gibb */ @Order(Ordered.LOWEST_PRECEDENCE - 100) -public class EnableDiscoveryClientImportSelector extends - SpringFactoryImportSelector { +public class EnableDiscoveryClientImportSelector + extends SpringFactoryImportSelector { @Override protected boolean isEnabled() { @@ -34,4 +34,9 @@ public class EnableDiscoveryClientImportSelector extends "spring.cloud.discovery.enabled", Boolean.class, Boolean.TRUE); } + @Override + protected boolean hasDefaultFactory() { + return true; + } + } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/util/SpringFactoryImportSelector.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/util/SpringFactoryImportSelector.java index 0d1450d2..39a71de8 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/util/SpringFactoryImportSelector.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/util/SpringFactoryImportSelector.java @@ -20,8 +20,6 @@ import java.util.ArrayList; import java.util.LinkedHashSet; import java.util.List; -import lombok.extern.apachecommons.CommonsLog; - import org.springframework.beans.factory.BeanClassLoaderAware; import org.springframework.context.EnvironmentAware; import org.springframework.context.annotation.DeferredImportSelector; @@ -32,16 +30,18 @@ import org.springframework.core.io.support.SpringFactoriesLoader; import org.springframework.core.type.AnnotationMetadata; import org.springframework.util.Assert; +import lombok.extern.apachecommons.CommonsLog; + /** - * Selects configurations to load defined by the generic type T. Loads - * implementations using {@link SpringFactoriesLoader}. + * Selects configurations to load defined by the generic type T. Loads implementations + * using {@link SpringFactoriesLoader}. * * @author Spencer Gibb * @author Dave Syer */ @CommonsLog -public abstract class SpringFactoryImportSelector implements - DeferredImportSelector, BeanClassLoaderAware, EnvironmentAware { +public abstract class SpringFactoryImportSelector + implements DeferredImportSelector, BeanClassLoaderAware, EnvironmentAware { private ClassLoader beanClassLoader; @@ -51,8 +51,8 @@ public abstract class SpringFactoryImportSelector implements @SuppressWarnings("unchecked") protected SpringFactoryImportSelector() { - this.annotationClass = (Class) GenericTypeResolver.resolveTypeArgument( - this.getClass(), SpringFactoryImportSelector.class); + this.annotationClass = (Class) GenericTypeResolver + .resolveTypeArgument(this.getClass(), SpringFactoryImportSelector.class); } @Override @@ -60,24 +60,24 @@ public abstract class SpringFactoryImportSelector implements if (!isEnabled()) { return new String[0]; } - AnnotationAttributes attributes = AnnotationAttributes.fromMap(metadata - .getAnnotationAttributes(this.annotationClass.getName(), true)); + AnnotationAttributes attributes = AnnotationAttributes.fromMap( + metadata.getAnnotationAttributes(this.annotationClass.getName(), true)); Assert.notNull(attributes, "No " + getSimpleName() + " attributes found. Is " + metadata.getClassName() + " annotated with @" + getSimpleName() + "?"); // Find all possible auto configuration classes, filtering duplicates - List factories = new ArrayList<>(new LinkedHashSet<>( - SpringFactoriesLoader.loadFactoryNames(this.annotationClass, - this.beanClassLoader))); + List factories = new ArrayList<>(new LinkedHashSet<>(SpringFactoriesLoader + .loadFactoryNames(this.annotationClass, this.beanClassLoader))); - if (factories.isEmpty()) { - throw new IllegalStateException("Annotation @" + getSimpleName() + - " found, but there are no implementations. Did you forget to include a starter?"); + if (factories.isEmpty() && !hasDefaultFactory()) { + throw new IllegalStateException("Annotation @" + getSimpleName() + + " found, but there are no implementations. Did you forget to include a starter?"); } if (factories.size() > 1) { - // there should only ever be one DiscoveryClient, but there might be more than one factory + // there should only ever be one DiscoveryClient, but there might be more than + // one factory log.warn("More than one implementation " + "of @" + getSimpleName() + " (now relying on @Conditionals to pick one): " + factories); } @@ -85,6 +85,10 @@ public abstract class SpringFactoryImportSelector implements return factories.toArray(new String[factories.size()]); } + protected boolean hasDefaultFactory() { + return false; + } + protected abstract boolean isEnabled(); protected String getSimpleName() {