From dab8f973a577d9293e8c6620f0046aa90c1df9fa Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Mon, 19 Mar 2018 12:17:08 -0400 Subject: [PATCH] Returns empty list when zookeeper throws NoNodeException. This will allow apps that use DiscoveryClient to continue to function as services register. This is a common occurrence in testing situations. Fixes gh-164 --- .../discovery/ZookeeperDiscoveryClient.java | 16 ++++++++-- .../ZookeeperDiscoveryClientTests.java | 29 +++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/ZookeeperDiscoveryClient.java b/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/ZookeeperDiscoveryClient.java index ee923c35..853321d7 100644 --- a/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/ZookeeperDiscoveryClient.java +++ b/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/ZookeeperDiscoveryClient.java @@ -25,9 +25,9 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.curator.x.discovery.ServiceDiscovery; import org.apache.curator.x.discovery.ServiceInstance; +import org.apache.zookeeper.KeeperException; import org.springframework.cloud.client.discovery.DiscoveryClient; import org.springframework.cloud.zookeeper.discovery.dependency.ZookeeperDependencies; -import org.springframework.util.ReflectionUtils; import static org.springframework.util.ReflectionUtils.rethrowRuntimeException; @@ -91,8 +91,14 @@ public class ZookeeperDiscoveryClient implements DiscoveryClient { instances.add(createServiceInstance(serviceIdToQuery, instance)); } return instances; + } catch (KeeperException.NoNodeException e) { + if (log.isDebugEnabled()) { + log.debug("Error getting instances from zookeeper. Possibly, no service has registered.", e); + } + // this means that nothing has registered as a service yes + return Collections.emptyList(); } catch (Exception exception) { - ReflectionUtils.rethrowRuntimeException(exception); + rethrowRuntimeException(exception); } return new ArrayList<>(); } @@ -124,6 +130,12 @@ public class ZookeeperDiscoveryClient implements DiscoveryClient { } try { services = new ArrayList<>(getServiceDiscovery().queryForNames()); + } catch (KeeperException.NoNodeException e) { + if (log.isDebugEnabled()) { + log.debug("Error getting instances from zookeeper. Possibly, no service has registered.", e); + } + // this means that nothing has registered as a service yes + return Collections.emptyList(); } catch (Exception e) { rethrowRuntimeException(e); diff --git a/spring-cloud-zookeeper-discovery/src/test/java/org/springframework/cloud/zookeeper/discovery/ZookeeperDiscoveryClientTests.java b/spring-cloud-zookeeper-discovery/src/test/java/org/springframework/cloud/zookeeper/discovery/ZookeeperDiscoveryClientTests.java index 0559f668..1b6b3523 100644 --- a/spring-cloud-zookeeper-discovery/src/test/java/org/springframework/cloud/zookeeper/discovery/ZookeeperDiscoveryClientTests.java +++ b/spring-cloud-zookeeper-discovery/src/test/java/org/springframework/cloud/zookeeper/discovery/ZookeeperDiscoveryClientTests.java @@ -2,10 +2,15 @@ package org.springframework.cloud.zookeeper.discovery; import java.util.List; +import org.apache.curator.x.discovery.ServiceDiscovery; +import org.apache.zookeeper.KeeperException.NoNodeException; import org.junit.Test; +import org.springframework.cloud.client.ServiceInstance; + import static org.assertj.core.api.BDDAssertions.then; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; /** * @author Marcin Grzejszczak @@ -21,4 +26,28 @@ public class ZookeeperDiscoveryClientTests { // then: then(services).isEmpty(); } + + @Test + public void getServicesShouldReturnEmptyWhenNoNodeException() throws Exception { + // given: + ServiceDiscovery serviceDiscovery = mock(ServiceDiscovery.class); + when(serviceDiscovery.queryForNames()).thenThrow(new NoNodeException()); + ZookeeperDiscoveryClient discoveryClient = new ZookeeperDiscoveryClient(serviceDiscovery, null); + // when: + List services = discoveryClient.getServices(); + // then: + then(services).isEmpty(); + } + + @Test + public void getInstancesshouldReturnEmptyWhenNoNodeException() throws Exception { + // given: + ServiceDiscovery serviceDiscovery = mock(ServiceDiscovery.class); + when(serviceDiscovery.queryForInstances("myservice")).thenThrow(new NoNodeException()); + ZookeeperDiscoveryClient discoveryClient = new ZookeeperDiscoveryClient(serviceDiscovery, null); + // when: + List instances = discoveryClient.getInstances("myservice"); + // then: + then(instances).isEmpty(); + } }