Fix a few TODOs

3 files changed, 10 insertions, 20 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
index cb0c9528f82..8d91c93e283 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
@@ -249,10 +249,6 @@ public class ApplicationController {
         if ( ! (id.instance().value().equals("default") || id.instance().value().startsWith("default-pr"))) // TODO: Support instances properly
             throw new UnsupportedOperationException("Only the instance names 'default' and names starting with 'default-pr' are supported at the moment");
         try (Lock lock = lock(id)) {
-            // TODO: Throwing is duplicated below.
-            if (get(id).isPresent())
-                throw new IllegalArgumentException("An application with id '" + id + "' already exists");
-
             com.yahoo.vespa.hosted.controller.api.identifiers.ApplicationId.validate(id.application().value());
             
             Optional<Tenant> tenant = controller.tenants().tenant(new TenantId(id.tenant().value()));
@@ -586,18 +582,14 @@ public class ApplicationController {
         deploymentTrigger.triggerFromCompletion(report);
     }
 
-    // TODO: Collapse this method and the next
-    public void restart(DeploymentId deploymentId) {
-        try {
-            configserverClient.restart(deploymentId, Optional.empty());
-        }
-        catch (NoInstanceException e) {
-            throw new IllegalArgumentException("Could not restart " + deploymentId + ": No such deployment");
-        }
-    }
-    public void restartHost(DeploymentId deploymentId, Hostname hostname) {
+    /**
+     * Tells config server to schedule a restart of all nodes in this deployment
+     *
+     * @param hostname If non-empty, restart will only be scheduled for this host
+     */
+    public void restart(DeploymentId deploymentId, Optional<Hostname> hostname) {
         try {
-            configserverClient.restart(deploymentId, Optional.of(hostname));
+            configserverClient.restart(deploymentId, hostname);
         }
         catch (NoInstanceException e) {
             throw new IllegalArgumentException("Could not restart " + deploymentId + ": No such deployment");
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
index 4e27e296f21..f7810744b71 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
@@ -739,10 +739,8 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         DeploymentId deploymentId = new DeploymentId(ApplicationId.from(tenantName, applicationName, instanceName),
                                                      new Zone(Environment.from(environment), RegionName.from(region)));
         // TODO: Propagate all filters
-        if (request.getProperty("hostname") != null)
-            controller.applications().restartHost(deploymentId, new Hostname(request.getProperty("hostname")));
-        else
-            controller.applications().restart(deploymentId);
+        Optional<Hostname> hostname = Optional.ofNullable(request.getProperty("hostname")).map(Hostname::new);
+        controller.applications().restart(deploymentId, hostname);
 
         // TODO: Change to return JSON
         return new StringResponse("Requested restart of " + path(TenantResource.API_PATH, tenantName,
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
index 250299d5b7f..d12f78a1db3 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
@@ -485,7 +485,7 @@ public class ApplicationApiTest extends ControllerContainerTest {
         // Create the same application again
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1", POST)
                                       .userIdentity(USER_ID),
-                              "{\"error-code\":\"BAD_REQUEST\",\"message\":\"An application with id 'tenant1.application1' already exists\"}",
+                              "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Could not create 'tenant1.application1': Application already exists\"}",
                               400);
 
         ConfigServerClientMock configServer = (ConfigServerClientMock)container.components().getComponent("com.yahoo.vespa.hosted.controller.ConfigServerClientMock");