diff --git a/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/DependencyPathUtils.java b/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/DependencyPathUtils.java new file mode 100644 index 00000000..9744c801 --- /dev/null +++ b/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/DependencyPathUtils.java @@ -0,0 +1,28 @@ +package org.springframework.cloud.zookeeper.discovery; + +/** + * Utils for correct dependency path format + * + * @author Denis Stepanov + * @since 1.0.4 + */ +public class DependencyPathUtils { + + /** + * Sanitizes path by ensuring that path starts with a slash and doesn't have one at the end + * @param path + * @return + */ + public static String sanitize(String path) { + return withLeadingSlash(withoutSlashAtEnd(path)); + } + + private static String withLeadingSlash(String value) { + return value.startsWith("/") ? value : "/" + value; + } + + private static String withoutSlashAtEnd(String value) { + return value.endsWith("/") ? value.substring(0, value.length() - 1) : value; + } + +} diff --git a/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/ZookeeperDiscoveryProperties.java b/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/ZookeeperDiscoveryProperties.java index dca5e332..87902bac 100644 --- a/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/ZookeeperDiscoveryProperties.java +++ b/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/ZookeeperDiscoveryProperties.java @@ -22,6 +22,8 @@ import java.util.Map; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.cloud.commons.util.InetUtils; +import javax.annotation.PostConstruct; + /** * Properties related to Zookeeper's Service Discovery. * @@ -73,6 +75,11 @@ public class ZookeeperDiscoveryProperties { this.instanceHost = this.hostInfo.getIpAddress(); } + @PostConstruct + public void init() { + this.root = DependencyPathUtils.sanitize(this.root); + } + public boolean isEnabled() { return this.enabled; } diff --git a/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/ZookeeperServiceInstances.java b/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/ZookeeperServiceInstances.java index 76aa090a..5d3b1c4e 100644 --- a/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/ZookeeperServiceInstances.java +++ b/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/ZookeeperServiceInstances.java @@ -10,6 +10,8 @@ import org.apache.commons.logging.LogFactory; import org.apache.curator.x.discovery.ServiceInstance; import org.springframework.cloud.zookeeper.discovery.dependency.ZookeeperDependencies; +import static org.springframework.cloud.zookeeper.discovery.DependencyPathUtils.sanitize; + /** * An {@link Iterable} representing registered Zookeeper instances. If using * {@link ZookeeperDependencies} it will return a list of registered Zookeeper instances @@ -37,8 +39,7 @@ public class ZookeeperServiceInstances implements Iterable> getZookeeperInstances() { ArrayList> allInstances = new ArrayList<>(); try { - Collection names = getNamesToQuery(); - for (String name : names) { + for (String name : getNamesToQuery()) { allInstances.addAll(nestedInstances(allInstances, name)); } return allInstances; @@ -66,27 +67,23 @@ public class ZookeeperServiceInstances implements Iterable> tryToGetInstances(String path) { try { - return this.serviceDiscovery - .getServiceDiscovery().queryForInstances(path.substring( - this.zookeeperDiscoveryProperties.getRoot().length())); + return this.serviceDiscovery.getServiceDiscovery().queryForInstances(getPathWithoutRoot(path)); } catch (Exception e) { log.trace("Exception occurred while trying to retrieve instances of [" + path + "]", e); return null; } } + private String getPathWithoutRoot(String path) { + return path.substring(this.zookeeperDiscoveryProperties.getRoot().length()); + } + private List> injectZookeeperServiceInstances( List> accumulator, String name) throws Exception { Collection> instances = this.serviceDiscovery @@ -116,7 +113,11 @@ public class ZookeeperServiceInstances implements Iterable getNamesToQuery() throws Exception { if (this.zookeeperDependencies == null) { - return this.serviceDiscovery.getServiceDiscovery().queryForNames(); + List names = new ArrayList<>(); + for (String name : this.serviceDiscovery.getServiceDiscovery().queryForNames()) { + names.add(sanitize(name)); + } + return names; } return this.zookeeperDependencies.getDependencyNames(); } @@ -125,4 +126,5 @@ public class ZookeeperServiceInstances implements Iterable> iterator() { return this.allInstances.iterator(); } + } diff --git a/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/dependency/ZookeeperDependencies.java b/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/dependency/ZookeeperDependencies.java index 164db8ef..cce3b7ae 100644 --- a/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/dependency/ZookeeperDependencies.java +++ b/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/dependency/ZookeeperDependencies.java @@ -27,6 +27,8 @@ import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.cloud.zookeeper.discovery.dependency.StubsConfiguration.DependencyPath; import org.springframework.util.StringUtils; +import static org.springframework.cloud.zookeeper.discovery.DependencyPathUtils.sanitize; + /** * Representation of this service's dependencies in Zookeeper * @@ -55,8 +57,8 @@ public class ZookeeperDependencies { @PostConstruct public void init() { - if (StringUtils.hasText(this.prefix) && !this.prefix.endsWith("/")) { - this.prefix = this.prefix + "/"; + if (StringUtils.hasText(this.prefix)) { + this.prefix = sanitize(this.prefix); } for (Map.Entry entry : this.dependencies.entrySet()) { ZookeeperDependency value = entry.getValue(); @@ -65,6 +67,8 @@ public class ZookeeperDependencies { value.setPath(entry.getKey()); } + value.setPath(sanitize(value.getPath())); + if (StringUtils.hasText(this.prefix)) { value.setPath(this.prefix + value.getPath()); } diff --git a/spring-cloud-zookeeper-discovery/src/test/java/org/springframework/cloud/zookeeper/discovery/ZookeeperDiscoveryPropertiesTest.java b/spring-cloud-zookeeper-discovery/src/test/java/org/springframework/cloud/zookeeper/discovery/ZookeeperDiscoveryPropertiesTest.java new file mode 100644 index 00000000..75dfbb77 --- /dev/null +++ b/spring-cloud-zookeeper-discovery/src/test/java/org/springframework/cloud/zookeeper/discovery/ZookeeperDiscoveryPropertiesTest.java @@ -0,0 +1,38 @@ +package org.springframework.cloud.zookeeper.discovery; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.springframework.cloud.commons.util.InetUtils; +import org.springframework.cloud.commons.util.InetUtilsProperties; + +import java.util.Arrays; + +import static org.assertj.core.api.BDDAssertions.then; + +@RunWith(Parameterized.class) +public class ZookeeperDiscoveryPropertiesTest { + + private String root; + + public ZookeeperDiscoveryPropertiesTest(String root) { + this.root = root; + } + + @Parameterized.Parameters(name = "With root {0}") + public static Iterable rootVariations() { + return Arrays.asList("es", "es/","/es"); + } + + @Test + public void should_escape_root() { + // given: + ZookeeperDiscoveryProperties zookeeperDiscoveryProperties = new ZookeeperDiscoveryProperties(new InetUtils(new InetUtilsProperties())); + // when: + zookeeperDiscoveryProperties.setRoot(root); + zookeeperDiscoveryProperties.init(); + // then: + then(zookeeperDiscoveryProperties.getRoot()).isEqualTo("/es"); + } + +} \ No newline at end of file diff --git a/spring-cloud-zookeeper-discovery/src/test/java/org/springframework/cloud/zookeeper/discovery/dependency/ZookeeperDependenciesTest.java b/spring-cloud-zookeeper-discovery/src/test/java/org/springframework/cloud/zookeeper/discovery/dependency/ZookeeperDependenciesTest.java new file mode 100644 index 00000000..e561515c --- /dev/null +++ b/spring-cloud-zookeeper-discovery/src/test/java/org/springframework/cloud/zookeeper/discovery/dependency/ZookeeperDependenciesTest.java @@ -0,0 +1,51 @@ +package org.springframework.cloud.zookeeper.discovery.dependency; + +import org.junit.Test; + +import java.util.LinkedHashMap; +import java.util.Map; + +import static org.assertj.core.api.BDDAssertions.then; + +public class ZookeeperDependenciesTest { + + @Test + public void should_properly_sanitize_dependency_path() { + // given: + Map dependencies = new LinkedHashMap<>(); + ZookeeperDependency cat = new ZookeeperDependency(); + cat.setPath("/cats/cat"); + dependencies.put("cat", cat); + ZookeeperDependency dog = new ZookeeperDependency(); + dog.setPath("dogs/dog"); + dependencies.put("dog", dog); + ZookeeperDependencies zookeeperDependencies = new ZookeeperDependencies(); + zookeeperDependencies.setDependencies(dependencies); + // when: + zookeeperDependencies.init(); + // then: + then(zookeeperDependencies.getDependencies().get("cat").getPath()).isEqualTo("/cats/cat"); + then(zookeeperDependencies.getDependencies().get("dog").getPath()).isEqualTo("/dogs/dog"); + } + + @Test + public void should_properly_sanitize_dependency_path_with_prefix() { + // given: + Map dependencies = new LinkedHashMap<>(); + ZookeeperDependency cat = new ZookeeperDependency(); + cat.setPath("/cats/cat"); + dependencies.put("cat", cat); + ZookeeperDependency dog = new ZookeeperDependency(); + dog.setPath("dogs/dog"); + dependencies.put("dog", dog); + ZookeeperDependencies zookeeperDependencies = new ZookeeperDependencies(); + zookeeperDependencies.setPrefix("animals/"); + zookeeperDependencies.setDependencies(dependencies); + // when: + zookeeperDependencies.init(); + // then: + then(zookeeperDependencies.getDependencies().get("cat").getPath()).isEqualTo("/animals/cats/cat"); + then(zookeeperDependencies.getDependencies().get("dog").getPath()).isEqualTo("/animals/dogs/dog"); + } + +} \ No newline at end of file diff --git a/spring-cloud-zookeeper-discovery/src/test/java/org/springframework/cloud/zookeeper/discovery/dependency/ZookeeperDiscoveryWithDependenciesIntegrationTests.java b/spring-cloud-zookeeper-discovery/src/test/java/org/springframework/cloud/zookeeper/discovery/dependency/ZookeeperDiscoveryWithDependenciesIntegrationTests.java index 2f38cd58..db96f77d 100644 --- a/spring-cloud-zookeeper-discovery/src/test/java/org/springframework/cloud/zookeeper/discovery/dependency/ZookeeperDiscoveryWithDependenciesIntegrationTests.java +++ b/spring-cloud-zookeeper-discovery/src/test/java/org/springframework/cloud/zookeeper/discovery/dependency/ZookeeperDiscoveryWithDependenciesIntegrationTests.java @@ -3,7 +3,6 @@ package org.springframework.cloud.zookeeper.discovery.dependency; import java.util.List; import java.util.concurrent.Callable; -import org.apache.curator.framework.CuratorFramework; import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; @@ -36,8 +35,6 @@ public class ZookeeperDiscoveryWithDependenciesIntegrationTests { @Autowired AliasUsingFeignClient aliasUsingFeignClient; @Autowired IdUsingFeignClient idUsingFeignClient; @Autowired ZookeeperDependencies zookeeperDependencies; - @Autowired Config dependencyConfig; - @Autowired CuratorFramework curatorFramework; @Test public void should_find_an_instance_via_path_when_alias_is_not_found() { // given: @@ -127,25 +124,25 @@ public class ZookeeperDiscoveryWithDependenciesIntegrationTests { }); } - @Test public void should_have_path_equal_to_alias() { + @Test public void should_have_path_equal_to_prefixed_alias() { // given: ZookeeperDependency dependency = this.zookeeperDependencies.getDependencyForAlias("aliasIsPath"); // expect: - then(dependency.getPath()).isEqualTo("aliasIsPath"); + then(dependency.getPath()).isEqualTo("/aliasIsPath"); } - @Test public void should_have_alias_equal_to_path() { + @Test public void should_have_prefixed_alias_equal_to_path() { // given: - ZookeeperDependency dependency = this.zookeeperDependencies.getDependencyForPath("aliasIsPath"); + ZookeeperDependency dependency = this.zookeeperDependencies.getDependencyForPath("/aliasIsPath"); // expect: - then(dependency.getPath()).isEqualTo("aliasIsPath"); + then(dependency.getPath()).isEqualTo("/aliasIsPath"); } @Test public void should_have_path_set_via_string_constructor() { // given: ZookeeperDependency dependency = this.zookeeperDependencies.getDependencyForAlias("anotherAlias"); // expect: - then(dependency.getPath()).isEqualTo("myPath"); + then(dependency.getPath()).isEqualTo("/myPath"); } private boolean callingServiceAtBeansEndpointIsNotEmpty() {