Improve coverage of cadi-client

Issue-ID: AAF-224
Change-Id: I950cffc889945205fada35b6f1d772c5318ba3b4
Signed-off-by: IanHowell <ian.howell@att.com>

5 files changed, 133 insertions, 55 deletions

diff --git a/cadi/client/src/main/java/org/onap/aaf/cadi/locator/DNSLocator.java b/cadi/client/src/main/java/org/onap/aaf/cadi/locator/DNSLocator.java
index 655a0c22..ed60b877 100644
--- a/cadi/client/src/main/java/org/onap/aaf/cadi/locator/DNSLocator.java
+++ b/cadi/client/src/main/java/org/onap/aaf/cadi/locator/DNSLocator.java
@@ -73,34 +73,7 @@ public class DNSLocator implements Locator<URI> {
 			throw new LocatorException("DNSLocator accepts only https or http protocols.  (requested URL " + aaf_locate + ')');
 		}
 		
-		int colon = aaf_locate.indexOf(':',start);
-		int slash;
-		if(colon>0) {
-			start = colon+1;
-			int left = aaf_locate.indexOf('[',start);
-			if(left>0) {
-				int right = aaf_locate.indexOf(']',left+1);
-				if(right>0) {
-					int dash = aaf_locate.indexOf('-',left+1);
-					if(dash<0) {
-						startPort = endPort = Integer.parseInt(aaf_locate.substring(left+1,right));
-					} else {
-						startPort = Integer.parseInt(aaf_locate.substring(left+1,dash));
-						endPort = Integer.parseInt(aaf_locate.substring(dash + 1,right));
-					}
-				}
-				
-			} else {
-				slash = aaf_locate.indexOf('/',colon+1);
-				if(slash<0) {
-					startPort = endPort = Integer.parseInt(aaf_locate.substring(start));
-				} else {
-					startPort = endPort = Integer.parseInt(aaf_locate.substring(start,slash));
-				}
-			}
-		} else {
-			startPort = endPort = port;
-		}		
+		parsePorts(aaf_locate.substring(start), port);
 	}
 
 	@Override
@@ -185,6 +158,46 @@ public class DNSLocator implements Locator<URI> {
 		}
 		return false;
 	}
+	
+	private void parsePorts(String aaf_locate, int defaultPort) throws LocatorException {
+		int slash, start;
+		int colon = aaf_locate.indexOf(':');
+		if(colon > 0) {
+			start = colon + 1;
+			int left = aaf_locate.indexOf('[', start);
+			if(left > 0) {
+				int right = aaf_locate.indexOf(']', left + 1);
+				if (right < 0) {
+					throw new LocatorException("Missing closing bracket in DNSLocator constructor.  (requested URL " + aaf_locate + ')');
+				} else if (right == (left + 1)) {
+					throw new LocatorException("Missing ports in brackets in DNSLocator constructor.  (requested URL " + aaf_locate + ')');
+				}
+				int dash = aaf_locate.indexOf('-', left + 1);
+				if (dash == (right - 1) || dash == (left + 1)) {
+					throw new LocatorException("Missing ports in brackets in DNSLocator constructor.  (requested URL " + aaf_locate + ')');
+				}
+				if(dash < 0) {
+					startPort = endPort = Integer.parseInt(aaf_locate.substring(left + 1, right));
+				} else {
+					startPort = Integer.parseInt(aaf_locate.substring(left + 1, dash));
+					endPort = Integer.parseInt(aaf_locate.substring(dash + 1, right));
+				}
+				
+			} else {
+				slash = aaf_locate.indexOf('/', start);
+				if (slash == start) {
+					throw new LocatorException("Missing port before '/' in DNSLocator constructor.  (requested URL " + aaf_locate + ')');
+				}
+				if(slash < 0) {
+					startPort = endPort = Integer.parseInt(aaf_locate.substring(start));
+				} else {
+					startPort = endPort = Integer.parseInt(aaf_locate.substring(start, slash));
+				}
+			}
+		} else {
+			startPort = endPort = defaultPort;
+		}		
+	}
 
 	private class Host {
 		private URI uri;
diff --git a/cadi/client/src/test/java/org/onap/aaf/cadi/http/test/JU_HBasicAuthSS.java b/cadi/client/src/test/java/org/onap/aaf/cadi/http/test/JU_HBasicAuthSS.java
index e6923ee1..1b9f6c3a 100644
--- a/cadi/client/src/test/java/org/onap/aaf/cadi/http/test/JU_HBasicAuthSS.java
+++ b/cadi/client/src/test/java/org/onap/aaf/cadi/http/test/JU_HBasicAuthSS.java
@@ -26,9 +26,7 @@ import java.io.IOException;
 import java.io.PrintStream;
 import java.net.HttpURLConnection;
 
-import static org.junit.Assert.*;
 import static org.mockito.Mockito.*;
-import static org.hamcrest.CoreMatchers.*;
 
 import org.junit.*;
 import org.mockito.*;
@@ -67,6 +65,7 @@ public class JU_HBasicAuthSS {
 	@Test
 	public void test() throws IOException {
 		// All the constructors accomplish the same thing
+		@SuppressWarnings("unused")
 		HBasicAuthSS auth = new HBasicAuthSS(si);
 		
 		// TODO: While these test _should_ pass, and they _do_ pass on my local machine, they won't
diff --git a/cadi/client/src/test/java/org/onap/aaf/cadi/http/test/JU_HClient.java b/cadi/client/src/test/java/org/onap/aaf/cadi/http/test/JU_HClient.java
index 0b4b8e78..646d63fa 100644
--- a/cadi/client/src/test/java/org/onap/aaf/cadi/http/test/JU_HClient.java
+++ b/cadi/client/src/test/java/org/onap/aaf/cadi/http/test/JU_HClient.java
@@ -25,15 +25,12 @@ import static org.junit.Assert.*;
 import static org.mockito.Mockito.*;
 
 import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
-import java.io.Reader;
 import java.lang.reflect.Field;
 import java.net.HttpURLConnection;
 import java.net.URI;
 import java.net.URISyntaxException;
-import java.net.URL;
 
 import javax.servlet.ServletOutputStream;
 import javax.servlet.http.HttpServletResponse;
diff --git a/cadi/client/src/test/java/org/onap/aaf/cadi/locator/test/JU_DNSLocator.java b/cadi/client/src/test/java/org/onap/aaf/cadi/locator/test/JU_DNSLocator.java
index 20e31347..a80e52f7 100644
--- a/cadi/client/src/test/java/org/onap/aaf/cadi/locator/test/JU_DNSLocator.java
+++ b/cadi/client/src/test/java/org/onap/aaf/cadi/locator/test/JU_DNSLocator.java
@@ -21,35 +21,105 @@
 
 package org.onap.aaf.cadi.locator.test;
 
+import static org.junit.Assert.*;
+import static org.hamcrest.CoreMatchers.*;
+import org.junit.*;
+
+import java.io.ByteArrayOutputStream;
+import java.io.PrintStream;
 import java.net.URI;
-import java.net.URL;
-import java.net.URLConnection;
 
-import org.junit.AfterClass;
-import org.junit.Test;
+import org.onap.aaf.cadi.LocatorException;
 import org.onap.aaf.cadi.PropAccess;
 import org.onap.aaf.cadi.Locator.Item;
 import org.onap.aaf.cadi.locator.DNSLocator;
 
 public class JU_DNSLocator {
-
-	@AfterClass
-	public static void tearDownAfterClass() throws Exception {
+	
+	private PropAccess access;
+	
+	@Before
+	public void setup() {
+		access = new PropAccess(new PrintStream(new ByteArrayOutputStream()), new String[0]);
 	}
 
 	@Test
-	public void test() {
-		// TODO: Actually test this class - Ian
+	public void test() throws LocatorException {
+		DNSLocator dl;
+		Item item;
+		URI uri;
+
+		dl = new DNSLocator(access, "https", "localhost", "8100-8101");
 		
-		DNSLocator dl = new DNSLocator(new PropAccess(), "https", "aaf.it.att.com","8150-8152");
-//		try {
-//			Item item = dl.best();
-//			URI uri = dl.get(item);
-//			URL url = uri.toURL();
-//			URLConnection conn = url.openConnection();
-//			conn.connect();
-//		} catch (Exception e) {
-//		}
+		item = dl.best();
+		uri = dl.get(item);
+		assertThat(uri.toString(), is("https://127.0.0.1:8100"));
+		item = dl.best();
+		assertThat(uri.toString(), is("https://127.0.0.1:8100"));
+
+		assertThat(dl.hasItems(), is(true));
+		for (item = dl.first(); item != null; item = dl.next(item)) {
+			dl.invalidate(item);
+		}
+		assertThat(dl.hasItems(), is(false));
+
+		// This doesn't actually do anything besides increase coverage 
+		dl.destroy();
+	}
+	
+	@Test
+	public void constructorTest() throws LocatorException {
+		// For coverage
+		new DNSLocator(access, "https", "localhost", "8100");
+		new DNSLocator(access, "https", "localhost", "8100-8101");
+
+		new DNSLocator(access, "http:localhost");
+		new DNSLocator(access, "https:localhost");
+		new DNSLocator(access, "https:localhost:8100");
+		new DNSLocator(access, "https:localhost:[8100]");
+		new DNSLocator(access, "https:localhost:[8100-8101]");
+		new DNSLocator(access, "https:localhost:8000/");
+	}
+	
+	@Test
+	public void refreshTest() throws LocatorException {
+		DNSLocator dl = new DNSLocator(access, "https", "bogushost", "8100-8101");
+		assertThat(dl.refresh(), is(false));
+	}
+	
+	@Test(expected = LocatorException.class)
+	public void throws1Test() throws LocatorException {
+		new DNSLocator(access, null);
+	}
+
+	@Test(expected = LocatorException.class)
+	public void throws2Test() throws LocatorException {
+		new DNSLocator(access, "ftp:invalid");
+	}
+
+	@Test(expected = LocatorException.class)
+	public void throws3Test() throws LocatorException {
+		new DNSLocator(access, "https:localhost:[8100");
+	}
+
+	@Test(expected = LocatorException.class)
+	public void throws4Test() throws LocatorException {
+		new DNSLocator(access, "https:localhost:[]");
+	}
+
+	@Test(expected = LocatorException.class)
+	public void throws5Test() throws LocatorException {
+		new DNSLocator(access, "https:localhost:[8100-]");
+	}
+
+	@Test(expected = LocatorException.class)
+	public void throws6Test() throws LocatorException {
+		new DNSLocator(access, "https:localhost:[-8101]");
+	}
+
+	@Test(expected = LocatorException.class)
+	public void throws7Test() throws LocatorException {
+		new DNSLocator(access, "https:localhost:/");
 	}
 
 }
diff --git a/cadi/client/src/test/java/org/onap/aaf/cadi/locator/test/JU_HClientHotPeerLocator.java b/cadi/client/src/test/java/org/onap/aaf/cadi/locator/test/JU_HClientHotPeerLocator.java
index 4b008c2c..1478cafe 100644
--- a/cadi/client/src/test/java/org/onap/aaf/cadi/locator/test/JU_HClientHotPeerLocator.java
+++ b/cadi/client/src/test/java/org/onap/aaf/cadi/locator/test/JU_HClientHotPeerLocator.java
@@ -22,7 +22,6 @@
 package org.onap.aaf.cadi.locator.test;
 
 import static org.junit.Assert.*;
-import static org.mockito.Mockito.*;
 import static org.hamcrest.CoreMatchers.*;
 import org.junit.*;
 import org.mockito.*;
@@ -87,10 +86,10 @@ public class JU_HClientHotPeerLocator {
 
 		item = loc.first();
 		loc.invalidate(item);
-		loc.get(item);
 		
+		loc.invalidate(loc.bestClient());
+		loc.invalidate(loc.get(loc.next(item)));
 		loc.destroy();
-		item = loc.best();
 	}
 
 	@Test(expected = LocatorException.class)
@@ -111,7 +110,7 @@ public class JU_HClientHotPeerLocator {
 		loc.invalidate(loc.first());
 
 		loc.destroy();
-		Locator.Item item = loc.best();
+		loc.best();
 	}
 	
 	@Test