Be explicit and assert on ApplicationInstanceReference format in

conversion to ApplicationId.

Modified test data and tests to reflect this.

4 files changed, 40 insertions, 30 deletions

diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorUtil.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorUtil.java
index 7782702995d..3f5df12723d 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorUtil.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorUtil.java
@@ -16,8 +16,8 @@ import com.yahoo.vespa.orchestrator.status.HostStatus;
 import com.yahoo.vespa.orchestrator.status.ReadOnlyStatusRegistry;
 
 import java.util.Collection;
+import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import java.util.Set;
 import java.util.function.Function;
 import java.util.regex.Matcher;
@@ -113,34 +113,44 @@ public class OrchestratorUtil {
                                                                               InstanceLookupService instanceLookupService)
             throws ApplicationIdNotFoundException {
 
-        String appRegex = String.format("^%s:.*:.*:%s$",appId.application().toString(), appId.instance().toString());
         Set<ApplicationInstanceReference> appRefs = instanceLookupService.knownInstances();
-        Optional<ApplicationInstanceReference> appRef = appRefs.stream()
-                .filter(a -> a.tenantId().equals(appId.tenant()))
-                .filter(a -> a.applicationInstanceId().toString().matches(appRegex))
-                .findFirst();
-        return appRef.orElseThrow(() -> new ApplicationIdNotFoundException());
-    }
+        List<ApplicationInstanceReference> appRefList = appRefs.stream()
+                .filter(a -> matchAppInstanceAndId(a, appId))
+                .collect(Collectors.toList());
 
-    public static ApplicationId toApplicationId(ApplicationInstanceReference appRef) {
-        TenantName tenantName = TenantName.from(appRef.tenantId().toString());
+        if (appRefList.size() != 1) {
+            throw new ApplicationIdNotFoundException();
+        }
 
-        // Now for the application/instance pair we need to split this
-        String appNameStr = appRef.applicationInstanceId().toString();
-        String[] appNameParts = appNameStr.split(":");
+        return appRefList.get(0);
+    }
 
-        // We assume a valid application reference has at lest two parts appname:instancename
-        if (appNameParts.length < 2)  {
-            throw new IllegalArgumentException("Application reference not valid: " + appRef);
+    private static boolean matchAppInstanceAndId(ApplicationInstanceReference appInstanceRef, ApplicationId appId) {
+        // Match tenant first
+        if (!appInstanceRef.tenantId().toString().equals(appId.tenant().toString())) {
+            return false;
         }
 
-        // Last part of string is the instance name
-        InstanceName instanceName = InstanceName.from(appNameParts[appNameParts.length-1]);
+        // Match appId and InstanceId
+        String[] appInstancePart = appInstanceRef.applicationInstanceId().toString().split(":");
+        if (appInstancePart.length != 4) {
+            throw new RuntimeException("Found inconsistent");
+        }
+        return appId.application().toString().equals(appInstancePart[0])
+                && appId.instance().toString().equals(appInstancePart[3]);
+    }
+
+    public static ApplicationId toApplicationId(ApplicationInstanceReference appRef) {
 
-        // The rest is application
-        int whereAppNameEnds = appNameStr.lastIndexOf(":");
-        ApplicationName appName = ApplicationName.from(appNameStr.substring(0, whereAppNameEnds));
+        String appNameStr = appRef.toString();
+        String[] appNameParts = appNameStr.split(":");
+        // This assumption will soon be validated in the AppRef model
+        if (appNameParts.length != 5)  {
+            throw new IllegalArgumentException("Application reference not valid (not 5 parts): " + appRef);
+        }
 
-        return ApplicationId.from(tenantName, appName, instanceName);
+        return ApplicationId.from(TenantName.from(appNameParts[0]),
+                ApplicationName.from(appNameParts[1]),
+                InstanceName.from(appNameParts[4]));
     }
 }
diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/DummyInstanceLookupService.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/DummyInstanceLookupService.java
index e1b73c1fe65..d48bc1f302b 100644
--- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/DummyInstanceLookupService.java
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/DummyInstanceLookupService.java
@@ -37,7 +37,7 @@ public class DummyInstanceLookupService implements InstanceLookupService {
     static {
         apps.add(new ApplicationInstance<>(
                 new TenantId("test-tenant-id"),
-                new ApplicationInstanceId("application:instance"),
+                new ApplicationInstanceId("application:prod:utopia-1:instance"),
                 TestUtil.makeServiceClusterSet(
                         new ServiceCluster<>(
                                 new ClusterId("test-cluster-id-1"),
@@ -49,7 +49,7 @@ public class DummyInstanceLookupService implements InstanceLookupService {
                                                 ServiceMonitorStatus.UP),
                                         new ServiceInstance<>(
                                                 new ConfigId("storage/storage/2"),
-                                                new HostName("test2.prod.utpoia-1.vespahosted.ut1.yahoo.com"),
+                                                new HostName("test2.prod.utopoia-1.vespahosted.ut1.yahoo.com"),
                                                 ServiceMonitorStatus.UP))),
                         new ServiceCluster<>(
                                 new ClusterId("clustercontroller"),
@@ -65,7 +65,7 @@ public class DummyInstanceLookupService implements InstanceLookupService {
 
         apps.add(new ApplicationInstance<>(
                 new TenantId("mediasearch"),
-                new ApplicationInstanceId("imagesearch:default"),
+                new ApplicationInstanceId("imagesearch:prod:utopia-1:default"),
                 TestUtil.makeServiceClusterSet(
                         new ServiceCluster<>(
                                 new ClusterId("image"),
@@ -93,7 +93,7 @@ public class DummyInstanceLookupService implements InstanceLookupService {
 
         apps.add(new ApplicationInstance<>(
                 new TenantId("tenant-id-3"),
-                new ApplicationInstanceId("application-instance-3:default"),
+                new ApplicationInstanceId("application-instance-3:prod:utopia-1:default"),
                 TestUtil.makeServiceClusterSet(
                         new ServiceCluster<>(
                                 new ClusterId("cluster-id-3"),
diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java
index 7f414dbfbc2..3adb65b9654 100644
--- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java
@@ -216,7 +216,7 @@ public class OrchestratorImplTest {
         InstanceLookupService service = new DummyInstanceLookupService();
         String applicationInstanceId = service.findInstanceByHost(DummyInstanceLookupService.TEST1_HOST_NAME).get()
                 .reference().toString();
-        assertEquals("test-tenant-id:application:instance", applicationInstanceId);
+        assertEquals("test-tenant-id:application:prod:utopia-1:instance", applicationInstanceId);
     }
 
     @Test
diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorUtilTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorUtilTest.java
index 178e39f6e5e..ba2a584c27c 100644
--- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorUtilTest.java
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorUtilTest.java
@@ -18,12 +18,12 @@ public class OrchestratorUtilTest {
 
     private static final ApplicationId APPID_1 = ApplicationId.from(
             TenantName.from("mediasearch"),
-            ApplicationName.from("tumblr-search"),
+            ApplicationName.from("imagesearch"),
             InstanceName.defaultName());
 
     private static final ApplicationInstanceReference APPREF_1 = new ApplicationInstanceReference(
-            new TenantId("test-tenant"),
-            new ApplicationInstanceId("test-application:test-environment:test-region:test-instance-key"));
+            new TenantId("test-tenant-id"),
+            new ApplicationInstanceId("application:prod:utopia-1:instance"));
 
     /**
      * Here we don't care how the internal of the different application