Sanitize path during initialization (#96)

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;
+	}
+
+}

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;
 	}

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<ServiceInstance<Zooke
 	private List<ServiceInstance<ZookeeperInstance>> getZookeeperInstances() {
 		ArrayList<ServiceInstance<ZookeeperInstance>> allInstances = new ArrayList<>();
 		try {
-			Collection<String> 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<ServiceInstance<Zooke
 	}
 
 	private String prepareQueryName(String name) {
-		if (name.startsWith(this.zookeeperDiscoveryProperties.getRoot())) {
-			return name;
-		}
-		String queryName = this.zookeeperDiscoveryProperties.getRoot();
-		if (!queryName.endsWith("/")) {
-			queryName = queryName + "/";
-		}
-		return queryName + name;
+		String root = this.zookeeperDiscoveryProperties.getRoot();
+		return name.startsWith(root) ? name : root + name;
 	}
 
 	private Collection<ServiceInstance<ZookeeperInstance>> 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<ServiceInstance<ZookeeperInstance>> injectZookeeperServiceInstances(
 			List<ServiceInstance<ZookeeperInstance>> accumulator, String name) throws Exception {
 		Collection<ServiceInstance<ZookeeperInstance>> instances = this.serviceDiscovery
@@ -116,7 +113,11 @@ public class ZookeeperServiceInstances implements Iterable<ServiceInstance<Zooke
 
 	private Collection<String> getNamesToQuery() throws Exception {
 		if (this.zookeeperDependencies == null) {
-			return this.serviceDiscovery.getServiceDiscovery().queryForNames();
+			List<String> 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<ServiceInstance<Zooke
 	public Iterator<ServiceInstance<ZookeeperInstance>> iterator() {
 		return this.allInstances.iterator();
 	}
+
 }

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<String, ZookeeperDependency> 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());
 			}

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<String> 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");
+    }
+
+}

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<String, ZookeeperDependency> 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<String, ZookeeperDependency> 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");
+    }
+
+}

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() {