Refactor prepare handler

Move more stuff into business layer, no functional changes

7 files changed, 110 insertions, 62 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
index b3c136d33ae..7d47cc0b101 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
@@ -17,6 +17,7 @@ import com.yahoo.config.provision.TenantName;
 import com.yahoo.container.jdisc.HttpResponse;
 import com.yahoo.log.LogLevel;
 import com.yahoo.path.Path;
+import com.yahoo.slime.Slime;
 import com.yahoo.transaction.NestedTransaction;
 import com.yahoo.vespa.config.server.application.Application;
 import com.yahoo.vespa.config.server.application.ApplicationConvergenceChecker;
@@ -26,8 +27,12 @@ import com.yahoo.vespa.config.server.application.HttpProxy;
 import com.yahoo.vespa.config.server.application.LogServerLogGrabber;
 import com.yahoo.vespa.config.server.application.TenantApplications;
 import com.yahoo.vespa.config.server.configchange.ConfigChangeActions;
+import com.yahoo.vespa.config.server.configchange.RefeedActions;
+import com.yahoo.vespa.config.server.configchange.RestartActions;
+import com.yahoo.vespa.config.server.deploy.DeployHandlerLogger;
 import com.yahoo.vespa.config.server.deploy.Deployment;
 import com.yahoo.vespa.config.server.http.SimpleHttpFetcher;
+import com.yahoo.vespa.config.server.http.v2.PrepareResult;
 import com.yahoo.vespa.config.server.provision.HostProvisionerProvider;
 import com.yahoo.vespa.config.server.session.LocalSession;
 import com.yahoo.vespa.config.server.session.LocalSessionRepo;
@@ -49,6 +54,7 @@ import java.util.Optional;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
+import java.util.logging.Level;
 import java.util.logging.Logger;
 
 /**
@@ -333,11 +339,17 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
         return currentActiveApplicationSet;
     }
 
-    public ConfigChangeActions prepare(Tenant tenant, long sessionId, DeployLogger logger, PrepareParams params) {
+    public PrepareResult prepare(Tenant tenant, long sessionId, PrepareParams prepareParams) {
+        validateThatLocalSessionIsNotActive(tenant, sessionId);
         LocalSession session = getLocalSession(tenant, sessionId);
-        ApplicationId appId = params.getApplicationId();
-        Optional<ApplicationSet> currentActiveApplicationSet = getCurrentActiveApplicationSet(tenant, appId);
-        return session.prepare(logger, params, currentActiveApplicationSet, tenant.getPath(), clock.instant());
+        ApplicationId applicationId = prepareParams.getApplicationId();
+        Optional<ApplicationSet> currentActiveApplicationSet = getCurrentActiveApplicationSet(tenant, applicationId);
+        Slime deployLog = createDeployLog();
+        DeployLogger logger = new DeployHandlerLogger(deployLog.get().setArray("log"), prepareParams.isVerbose(), applicationId);
+        ConfigChangeActions actions = session.prepare(logger, prepareParams, currentActiveApplicationSet, tenant.getPath(), clock.instant());
+        logConfigChangeActions(actions, logger);
+        log.log(LogLevel.INFO, Tenants.logPre(applicationId) + "Session " + sessionId + " prepared successfully. ");
+        return new PrepareResult(sessionId, actions, deployLog);
     }
 
     private List<ApplicationId> listApplicationIds(Tenant tenant) {
@@ -410,4 +422,25 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
         return null;
     }
 
+    private static void logConfigChangeActions(ConfigChangeActions actions, DeployLogger logger) {
+        RestartActions restartActions = actions.getRestartActions();
+        if ( ! restartActions.isEmpty()) {
+            logger.log(Level.WARNING, "Change(s) between active and new application that require restart:\n" +
+                    restartActions.format());
+        }
+        RefeedActions refeedActions = actions.getRefeedActions();
+        if ( ! refeedActions.isEmpty()) {
+            boolean allAllowed = refeedActions.getEntries().stream().allMatch(RefeedActions.Entry::allowed);
+            logger.log(allAllowed ? Level.INFO : Level.WARNING,
+                       "Change(s) between active and new application that may require re-feed:\n" +
+                               refeedActions.format());
+        }
+    }
+
+    private Slime createDeployLog() {
+        Slime deployLog = new Slime();
+        deployLog.setObject();
+        return deployLog;
+    }
+
 }
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/SessionHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/SessionHandler.java
index 4f7548d82ce..4d25ba9eeba 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/SessionHandler.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/SessionHandler.java
@@ -76,6 +76,7 @@ public class SessionHandler extends HttpHandler {
         return new DeployHandlerLogger(deployLog.get().setArray("log"), verbose, app);
     }
 
+    // TODO: Refactor to use the one in ApplicationRepository and remove
     protected Slime createDeployLog() {
         Slime deployLog = new Slime();
         deployLog.setObject();
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/PrepareResult.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/PrepareResult.java
new file mode 100644
index 00000000000..99320df11df
--- /dev/null
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/PrepareResult.java
@@ -0,0 +1,35 @@
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.config.server.http.v2;
+
+import com.yahoo.slime.Slime;
+import com.yahoo.vespa.config.server.configchange.ConfigChangeActions;
+
+/**
+ * Encapsulates the result from preparing an application
+ *
+ * @author hmusum
+ */
+public class PrepareResult {
+
+    private final long sessionId;
+    private final ConfigChangeActions configChangeActions;
+    private final Slime deployLog;
+
+    public PrepareResult(long sessionId, ConfigChangeActions configChangeActions, Slime deployLog) {
+        this.sessionId = sessionId;
+        this.configChangeActions = configChangeActions;
+        this.deployLog = deployLog;
+    }
+
+    public long sessionId() {
+        return sessionId;
+    }
+
+    public ConfigChangeActions configChangeActions() {
+        return configChangeActions;
+    }
+
+    public Slime deployLog() {
+        return deployLog;
+    }
+}
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandler.java
index 2b432a50ee1..c0782d11ad9 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandler.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandler.java
@@ -3,40 +3,25 @@ package com.yahoo.vespa.config.server.http.v2;
 
 import com.google.inject.Inject;
 import com.yahoo.cloud.config.ConfigserverConfig;
-import com.yahoo.config.application.api.DeployLogger;
-import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.TenantName;
 import com.yahoo.container.jdisc.HttpRequest;
 import com.yahoo.container.jdisc.HttpResponse;
-import com.yahoo.container.logging.AccessLog;
-import com.yahoo.log.LogLevel;
-import com.yahoo.slime.Slime;
 import com.yahoo.vespa.config.server.ApplicationRepository;
-import com.yahoo.vespa.config.server.configchange.RestartActions;
 import com.yahoo.vespa.config.server.session.PrepareParams;
 import com.yahoo.vespa.config.server.tenant.Tenant;
 import com.yahoo.vespa.config.server.tenant.Tenants;
-import com.yahoo.vespa.config.server.configchange.ConfigChangeActions;
-import com.yahoo.vespa.config.server.configchange.RefeedActions;
 import com.yahoo.vespa.config.server.http.SessionHandler;
 import com.yahoo.vespa.config.server.http.Utils;
 
 import java.time.Duration;
-import java.util.concurrent.Executor;
-import java.util.logging.Level;
-import java.util.logging.Logger;
 
 /**
  * A handler that prepares a session given by an id in the request. v2 of application API
  *
  * @author hmusum
- * @since 5.1.29
  */
-// TODO: Move business logic out of the HTTP layer and delegate to a ApplicationRepository
 public class SessionPrepareHandler extends SessionHandler {
 
-    private static final Logger log = Logger.getLogger(SessionPrepareHandler.class.getName());
-
     private final Tenants tenants;
     private final Duration zookeeperBarrierTimeout;
 
@@ -50,44 +35,14 @@ public class SessionPrepareHandler extends SessionHandler {
         this.zookeeperBarrierTimeout = Duration.ofSeconds(configserverConfig.zookeeper().barrierTimeout());
     }
 
-    @Override
-    public Duration getTimeout() {
-        return zookeeperBarrierTimeout.plus(Duration.ofSeconds(10));
-    }
-
-    @Override
+  @Override
     protected HttpResponse handlePUT(HttpRequest request) {
         Tenant tenant = getExistingTenant(request);
         TenantName tenantName = tenant.getName();
         long sessionId = getSessionIdV2(request);
-        applicationRepository.validateThatLocalSessionIsNotActive(tenant, sessionId);
         PrepareParams prepareParams = PrepareParams.fromHttpRequest(request, tenantName, zookeeperBarrierTimeout);
-        // An app id currently using only the name
-        ApplicationId appId = prepareParams.getApplicationId();
-        Slime rawDeployLog = createDeployLog();
-        DeployLogger logger = createLogger(rawDeployLog, request.getBooleanProperty("verbose"), appId);
-        ConfigChangeActions actions = applicationRepository.prepare(tenant,
-                                                                    sessionId,
-                                                                    logger,
-                                                                    prepareParams);
-        logConfigChangeActions(actions, logger);
-        log.log(LogLevel.INFO, Tenants.logPre(appId) + "Session " + sessionId + " prepared successfully. ");
-        return new SessionPrepareResponse(rawDeployLog, tenantName, request, sessionId, actions);
-    }
-
-    private static void logConfigChangeActions(ConfigChangeActions actions, DeployLogger logger) {
-        RestartActions restartActions = actions.getRestartActions();
-        if ( ! restartActions.isEmpty()) {
-            logger.log(Level.WARNING, "Change(s) between active and new application that require restart:\n" +
-                                      restartActions.format());
-        }
-        RefeedActions refeedActions = actions.getRefeedActions();
-        if ( ! refeedActions.isEmpty()) {
-            boolean allAllowed = refeedActions.getEntries().stream().allMatch(RefeedActions.Entry::allowed);
-            logger.log(allAllowed ? Level.INFO : Level.WARNING,
-                       "Change(s) between active and new application that may require re-feed:\n" +
-                       refeedActions.format());
-        }
+        PrepareResult result = applicationRepository.prepare(tenant, sessionId, prepareParams);
+        return new SessionPrepareResponse(result, tenantName, request);
     }
 
     @Override
@@ -99,6 +54,11 @@ public class SessionPrepareHandler extends SessionHandler {
         return new SessionPrepareResponse(createDeployLog(), tenant.getName(), request, sessionId);
     }
 
+    @Override
+    public Duration getTimeout() {
+        return zookeeperBarrierTimeout.plus(Duration.ofSeconds(10));
+    }
+
     private Tenant getExistingTenant(HttpRequest request) {
         TenantName tenantName = Utils.getTenantNameFromSessionRequest(request);
         Utils.checkThatTenantExists(tenants, tenantName);
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareResponse.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareResponse.java
index 902a7415643..6d2aa426036 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareResponse.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareResponse.java
@@ -12,15 +12,18 @@ import com.yahoo.vespa.config.server.http.SessionResponse;
  * Creates a response for SessionPrepareHandler.
  *
  * @author hmusum
- * @since 5.1.28
  */
 class SessionPrepareResponse extends SessionResponse {
 
-    public SessionPrepareResponse(Slime deployLog, TenantName tenantName, HttpRequest request, long sessionId) {
+    SessionPrepareResponse(Slime deployLog, TenantName tenantName, HttpRequest request, long sessionId) {
         this(deployLog, tenantName, request, sessionId, new ConfigChangeActions());
     }
 
-    public SessionPrepareResponse(Slime deployLog, TenantName tenantName, HttpRequest request, long sessionId, ConfigChangeActions actions) {
+    SessionPrepareResponse(PrepareResult result, TenantName tenantName, HttpRequest request) {
+        this(result.deployLog(), tenantName, request, result.sessionId(), result.configChangeActions());
+    }
+
+    private SessionPrepareResponse(Slime deployLog, TenantName tenantName, HttpRequest request, long sessionId, ConfigChangeActions actions) {
         super(deployLog, deployLog.get());
         String message = "Session " + sessionId + " for tenant '" + tenantName.value() + "' prepared.";
         this.root.setString("tenant", tenantName.value());
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/PrepareParams.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/PrepareParams.java
index cbcfb865acf..b53ee9a5988 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/PrepareParams.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/PrepareParams.java
@@ -26,6 +26,7 @@ public final class PrepareParams {
     static final String INSTANCE_PARAM_NAME = "instance";
     static final String IGNORE_VALIDATION_PARAM_NAME = "ignoreValidationErrors";
     static final String DRY_RUN_PARAM_NAME = "dryRun";
+    static final String VERBOSE_PARAM_NAME = "verbose";
     static final String VESPA_VERSION_PARAM_NAME = "vespaVersion";
     static final String ROTATIONS_PARAM_NAME = "rotations";
 
@@ -33,15 +34,17 @@ public final class PrepareParams {
     private final TimeoutBudget timeoutBudget;
     private final boolean ignoreValidationErrors;
     private final boolean dryRun;
+    private final boolean verbose;
     private final Optional<Version> vespaVersion;
     private final Set<Rotation> rotations;
 
     private PrepareParams(ApplicationId applicationId, TimeoutBudget timeoutBudget, boolean ignoreValidationErrors,
-                         boolean dryRun, Optional<Version> vespaVersion, Set<Rotation> rotations) {
+                         boolean dryRun, boolean verbose, Optional<Version> vespaVersion, Set<Rotation> rotations) {
         this.timeoutBudget = timeoutBudget;
         this.applicationId = applicationId;
         this.ignoreValidationErrors = ignoreValidationErrors;
         this.dryRun = dryRun;
+        this.verbose = verbose;
         this.vespaVersion = vespaVersion;
         this.rotations = rotations;
     }
@@ -50,6 +53,7 @@ public final class PrepareParams {
 
         private boolean ignoreValidationErrors = false;
         private boolean dryRun = false;
+        private boolean verbose = false;
         private ApplicationId applicationId = ApplicationId.defaultId();
         private TimeoutBudget timeoutBudget = new TimeoutBudget(Clock.systemUTC(), Duration.ofSeconds(30));
         private Optional<Version> vespaVersion = Optional.empty();
@@ -72,6 +76,11 @@ public final class PrepareParams {
             return this;
         }
 
+        public Builder verbose(boolean verbose) {
+            this.verbose = verbose;
+            return this;
+        }
+
         public Builder timeoutBudget(TimeoutBudget timeoutBudget) {
             this.timeoutBudget = timeoutBudget;
             return this;
@@ -99,14 +108,15 @@ public final class PrepareParams {
 
         public PrepareParams build() {
             return new PrepareParams(applicationId, timeoutBudget, ignoreValidationErrors, dryRun, 
-                                     vespaVersion, rotations);
+                                     verbose, vespaVersion, rotations);
         }
 
     }
 
     public static PrepareParams fromHttpRequest(HttpRequest request, TenantName tenant, Duration barrierTimeout) {
-        return new PrepareParams.Builder().ignoreValidationErrors(request.getBooleanProperty(IGNORE_VALIDATION_PARAM_NAME))
+        return new Builder().ignoreValidationErrors(request.getBooleanProperty(IGNORE_VALIDATION_PARAM_NAME))
                                           .dryRun(request.getBooleanProperty(DRY_RUN_PARAM_NAME))
+                                          .verbose(request.getBooleanProperty(VERBOSE_PARAM_NAME))
                                           .timeoutBudget(SessionHandler.getTimeoutBudget(request, barrierTimeout))
                                           .applicationId(createApplicationId(request, tenant))
                                           .vespaVersion(request.getProperty(VESPA_VERSION_PARAM_NAME))
@@ -151,6 +161,10 @@ public final class PrepareParams {
         return dryRun;
     }
 
+    public boolean isVerbose() {
+        return verbose;
+    }
+
     public TimeoutBudget getTimeoutBudget() {
         return timeoutBudget;
     }
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/PrepareParamsTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/PrepareParamsTest.java
index 2c5de7b4334..f2fb4aa1c40 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/PrepareParamsTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/PrepareParamsTest.java
@@ -32,17 +32,18 @@ public class PrepareParamsTest {
 
         assertThat(prepareParams.getApplicationId(), is(ApplicationId.defaultId()));
         assertFalse(prepareParams.isDryRun());
+        assertFalse(prepareParams.isVerbose());
         assertFalse(prepareParams.ignoreValidationErrors());
         assertThat(prepareParams.vespaVersion(), is(Optional.<String>empty()));
         assertTrue(prepareParams.getTimeoutBudget().hasTimeLeft());
         assertThat(prepareParams.rotations().size(), is(0));
     }
 
-
-    static final String rotation = "rotation-042.vespa.a02.yahoodns.net";
-    static final String vespaVersion = "6.37.49";
-    static final String request = "http://foo:19071/application/v2/tenant/foo/application/bar?" +
+    private static final String rotation = "rotation-042.vespa.a02.yahoodns.net";
+    private static final String vespaVersion = "6.37.49";
+    private static final String request = "http://foo:19071/application/v2/tenant/foo/application/bar?" +
             PrepareParams.DRY_RUN_PARAM_NAME + "=true&" +
+            PrepareParams.VERBOSE_PARAM_NAME+ "=true&" +
             PrepareParams.IGNORE_VALIDATION_PARAM_NAME + "=false&" +
             PrepareParams.APPLICATION_NAME_PARAM_NAME + "=baz&" +
             PrepareParams.VESPA_VERSION_PARAM_NAME + "=" + vespaVersion;
@@ -55,6 +56,7 @@ public class PrepareParamsTest {
 
         assertThat(prepareParams.getApplicationId().serializedForm(), is("foo:baz:default"));
         assertTrue(prepareParams.isDryRun());
+        assertTrue(prepareParams.isVerbose());
         assertFalse(prepareParams.ignoreValidationErrors());
         Version expectedVersion = Version.fromString(vespaVersion);
         assertThat(prepareParams.vespaVersion().get(), is(expectedVersion));