Cleanup validation of session status

6 files changed, 29 insertions, 22 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java
index 1fbc5928d3e..d5ac57e479e 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java
@@ -1,7 +1,6 @@
 // Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.config.server.deploy;
 
-import com.yahoo.cloud.config.ConfigserverConfig;
 import com.yahoo.config.application.api.DeployLogger;
 import com.yahoo.config.provision.HostFilter;
 import com.yahoo.config.provision.Provisioner;
@@ -160,7 +159,7 @@ public class Deployment implements com.yahoo.config.provision.Deployment {
         if (Session.Status.NEW.equals(localSession.getStatus())) {
             throw new IllegalStateException(localSession.logPre() + "Session " + sessionId + " is not prepared");
         } else if (Session.Status.ACTIVATE.equals(localSession.getStatus())) {
-            throw new IllegalArgumentException(localSession.logPre() + "Session " + sessionId + " is already active");
+            throw new IllegalStateException(localSession.logPre() + "Session " + sessionId + " is already active");
         }
         return sessionId;
     }
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 9ae2966c425..34e6371b213 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
@@ -60,9 +60,7 @@ public class SessionPrepareHandler extends SessionHandler {
         TenantName tenantName = tenant.getName();
         long sessionId = getSessionIdV2(request);
         LocalSession session = applicationRepository.getLocalSession(tenant, getSessionIdV2(request));
-        if (Session.Status.ACTIVATE.equals(session.getStatus())) {
-            throw new IllegalArgumentException("Session is active: " + sessionId);
-        }
+        validateThatSessionIsNotActive(session);
         log.log(LogLevel.DEBUG, "session=" + session);
         boolean verbose = request.getBooleanProperty("verbose");
         Slime rawDeployLog = createDeployLog();
@@ -95,15 +93,22 @@ public class SessionPrepareHandler extends SessionHandler {
     @Override
     protected HttpResponse handleGET(HttpRequest request) {
         Tenant tenant = getExistingTenant(request);
-        long sessionId = getSessionIdV2(request);
         RemoteSession session = applicationRepository.getRemoteSession(tenant, getSessionIdV2(request));
-        if (Session.Status.ACTIVATE.equals(session.getStatus()))
-            throw new IllegalArgumentException("Session is active: " + sessionId);
-        if (!Session.Status.PREPARE.equals(session.getStatus()))
-            throw new IllegalArgumentException("Session not prepared: " + sessionId);
+        validateThatSessionIsNotActive(session);
+        validateThatSessionIsPrepared(session);
         return new SessionPrepareResponse(createDeployLog(), tenant, request, session, new ConfigChangeActions());
     }
 
+    private void validateThatSessionIsNotActive(Session session) {
+        if (Session.Status.ACTIVATE.equals(session.getStatus()))
+            throw new IllegalStateException("Session is active: " + session.getSessionId());
+    }
+
+    private void validateThatSessionIsPrepared(Session session) {
+        if ( ! Session.Status.PREPARE.equals(session.getStatus()))
+            throw new IllegalStateException("Session not prepared: " + session.getSessionId());
+    }
+
     private static Optional<ApplicationSet> getCurrentActiveApplicationSet(Tenant tenant, ApplicationId appId) {
         Optional<ApplicationSet> currentActiveApplicationSet = Optional.empty();
         TenantApplications applicationRepo = tenant.getApplicationRepo();
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/LocalSession.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/LocalSession.java
index 48a2674f45d..a68ce0441ab 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/LocalSession.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/LocalSession.java
@@ -37,7 +37,6 @@ public class LocalSession extends Session implements Comparable<LocalSession> {
 
     private final ApplicationPackage applicationPackage;
     private final TenantApplications applicationRepo;
-    private final SessionZooKeeperClient zooKeeperClient;
     private final SessionPreparer sessionPreparer;
     private final SessionContext sessionContext;
     private final File serverDB;
@@ -50,10 +49,9 @@ public class LocalSession extends Session implements Comparable<LocalSession> {
      */
     // TODO tenant in SessionContext?
     public LocalSession(TenantName tenant, long sessionId, SessionPreparer sessionPreparer, SessionContext sessionContext) {
-        super(tenant, sessionId);
+        super(tenant, sessionId, sessionContext.getSessionZooKeeperClient());
         this.serverDB = sessionContext.getServerDBSessionDir();
         this.applicationPackage = sessionContext.getApplicationPackage();
-        this.zooKeeperClient = sessionContext.getSessionZooKeeperClient();
         this.applicationRepo = sessionContext.getApplicationRepo();
         this.sessionPreparer = sessionPreparer;
         this.sessionContext = sessionContext;
@@ -93,10 +91,6 @@ public class LocalSession extends Session implements Comparable<LocalSession> {
         return zooKeeperClient.createWriteStatusTransaction(status);
     }
 
-    public Session.Status getStatus() {
-        return zooKeeperClient.readStatus();
-    }
-
     private void setStatus(Session.Status newStatus) {
         zooKeeperClient.writeStatus(newStatus);
     }
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSession.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSession.java
index 9b8dcd58464..9f733f7ea9a 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSession.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSession.java
@@ -23,7 +23,6 @@ public class RemoteSession extends Session {
 
     private static final Logger log = Logger.getLogger(RemoteSession.class.getName());
     private volatile ApplicationSet applicationSet = null;
-    private final SessionZooKeeperClient zooKeeperClient;
     private final ActivatedModelsBuilder applicationLoader;
 
     /**
@@ -38,8 +37,7 @@ public class RemoteSession extends Session {
                          long sessionId,
                          GlobalComponentRegistry globalComponentRegistry,
                          SessionZooKeeperClient zooKeeperClient) {
-        super(tenant, sessionId);
-        this.zooKeeperClient = zooKeeperClient;
+        super(tenant, sessionId, zooKeeperClient);
         this.applicationLoader = new ActivatedModelsBuilder(tenant, sessionId, zooKeeperClient, globalComponentRegistry);
     }
 
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/Session.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/Session.java
index 74f3de18ffc..a77f3ee6b38 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/Session.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/Session.java
@@ -16,10 +16,12 @@ public abstract class Session {
 
     private final long sessionId;
     protected final TenantName tenant;
+    protected final SessionZooKeeperClient zooKeeperClient;
 
-    protected Session(TenantName tenant, long sessionId) {
+    protected Session(TenantName tenant, long sessionId, SessionZooKeeperClient zooKeeperClient) {
         this.tenant = tenant;
         this.sessionId = sessionId;
+        this.zooKeeperClient = zooKeeperClient;
     }
     /**
      * Retrieve the session id for this session.
@@ -29,6 +31,10 @@ public abstract class Session {
         return sessionId;
     }
 
+    public Session.Status getStatus() {
+        return zooKeeperClient.readStatus();
+    }
+
     @Override
     public String toString() {
         return "Session,id=" + sessionId;
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionRepoTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionRepoTest.java
index 96d0181161c..9587f7795b1 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionRepoTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionRepoTest.java
@@ -1,6 +1,9 @@
 // Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.config.server.session;
 
+import com.yahoo.path.Path;
+import com.yahoo.vespa.config.server.PathProvider;
+import com.yahoo.vespa.curator.mock.MockCurator;
 import org.junit.Test;
 
 import com.yahoo.config.provision.TenantName;
@@ -32,7 +35,9 @@ public class SessionRepoTest {
 
     private class TestSession extends Session {
         public TestSession(long sessionId) {
-            super(TenantName.from("default"), sessionId);
+            super(TenantName.from("default"),
+                  sessionId,
+                  new MockSessionZKClient(new MockCurator(), new PathProvider(Path.createRoot()).getSessionDir(sessionId)));
         }
     }
 }