Set sessions path correctly in test constructor

No functional changes, some other minor refactorings

5 files changed, 36 insertions, 39 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSessionRepo.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSessionRepo.java
index 2269a7ed997..41c4ddd676e 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSessionRepo.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSessionRepo.java
@@ -28,12 +28,11 @@ import org.apache.curator.framework.recipes.cache.*;
 /**
  * Will watch/prepare sessions (applications) based on watched nodes in ZooKeeper, set for example
  * by the prepare HTTP handler on another configserver. The zookeeper state watched in this class is shared
- * between all configservers, so it should not modify any global state, because the operation will be performed
+ * between all config servers, so it should not modify any global state, because the operation will be performed
  * on all servers. The repo can be regarded as read only from the POV of the configserver.
  *
- * @author vegardh
- * @author lulf
- * @since 5.1
+ * @author Vegard Havdal
+ * @author Ulf Lilleengen
  */
 public class RemoteSessionRepo extends SessionRepo<RemoteSession> implements NodeCacheListener, PathChildrenCacheListener {
 
@@ -74,6 +73,17 @@ public class RemoteSessionRepo extends SessionRepo<RemoteSession> implements Nod
         sessionsChanged();
     }
 
+    // For testing only
+    public RemoteSessionRepo(TenantName tenantName) {
+        this.curator = null;
+        this.remoteSessionFactory = null;
+        this.reloadHandler = null;
+        this.sessionsPath = Tenants.getSessionsPath(tenantName);
+        this.metrics = null;
+        this.directoryCache = null;
+        this.applicationRepo = null;
+    }
+
     //---------- START overrides to keep sessions changed in sync 
 
     @Override
@@ -122,17 +132,6 @@ public class RemoteSessionRepo extends SessionRepo<RemoteSession> implements Nod
         }
     }
 
-    // For testing only
-    public RemoteSessionRepo() {
-        this.curator = null;
-        this.remoteSessionFactory = null;
-        this.reloadHandler = null;
-        this.sessionsPath = Path.createRoot();
-        this.metrics = null;
-        this.directoryCache = null;
-        this.applicationRepo = null;
-    }
-
     private List<Long> getSessionList(List<ChildData> children) {
         List<Long> sessions = new ArrayList<>();
         for (ChildData data : children) {
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java
index 9d04b7e982d..6542c865d56 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java
@@ -84,16 +84,24 @@ public class SessionActiveHandlerTest extends SessionHandlerTest {
     private LocalSessionRepo localRepo;
     private TenantApplications applicationRepo;
     private MockProvisioner hostProvisioner;
+    private VespaModelFactory modelFactory;
+    private TestComponentRegistry componentRegistry;
 
     @Before
     public void setup() throws Exception {
-        remoteSessionRepo = new RemoteSessionRepo();
+        remoteSessionRepo = new RemoteSessionRepo(tenant);
         applicationRepo = new MemoryTenantApplications();
         curator = new MockCurator();
         configCurator = ConfigCurator.create(curator);
         localRepo = new LocalSessionRepo(Clock.systemUTC());
         pathPrefix = "/application/v2/tenant/" + tenant + "/session/";
         hostProvisioner = new MockProvisioner();
+        modelFactory = new VespaModelFactory(new NullConfigModelRegistry());
+        componentRegistry = new TestComponentRegistry.Builder()
+                .curator(curator)
+                .configCurator(configCurator)
+                .modelFactoryRegistry(new ModelFactoryRegistry(Collections.singletonList(modelFactory)))
+                .build();
     }
 
     @Test
@@ -211,14 +219,8 @@ public class SessionActiveHandlerTest extends SessionHandlerTest {
     private RemoteSession createRemoteSession(long sessionId, Session.Status status, SessionZooKeeperClient zkClient, Clock clock) throws IOException {
         zkClient.writeStatus(status);
         ZooKeeperClient zkC = new ZooKeeperClient(configCurator, new BaseDeployLogger(), false, Tenants.getSessionsPath(tenant).append(String.valueOf(sessionId)));
-        VespaModelFactory modelFactory = new VespaModelFactory(new NullConfigModelRegistry());
         zkC.write(Collections.singletonMap(modelFactory.getVersion(), new MockFileRegistry()));
         zkC.write(AllocatedHosts.withHosts(Collections.emptySet()));
-        TestComponentRegistry componentRegistry = new TestComponentRegistry.Builder()
-                .curator(curator)
-                .configCurator(configCurator)
-                .modelFactoryRegistry(new ModelFactoryRegistry(Collections.singletonList(modelFactory)))
-                .build();
         RemoteSession session = new RemoteSession(TenantName.from("default"), sessionId, componentRegistry, zkClient, clock);
         remoteSessionRepo.addSession(session);
         return session;
@@ -246,7 +248,7 @@ public class SessionActiveHandlerTest extends SessionHandlerTest {
         return activateRequest;
     }
 
-    private ActivateRequest activateAndAssertErrorPut(long sessionId, long previousSessionId, Clock clock,
+    private void activateAndAssertErrorPut(long sessionId, long previousSessionId, Clock clock,
                                                       int statusCode, HttpErrorResponse.errorCodes errorCode, String expectedError) throws Exception {
         ActivateRequest activateRequest = new ActivateRequest(sessionId, previousSessionId, "", clock);
         activateRequest.invoke();
@@ -256,7 +258,6 @@ public class SessionActiveHandlerTest extends SessionHandlerTest {
         String message = getRenderedString(actResponse);
         assertThat(message, Is.is("{\"error-code\":\"" + errorCode.name() + "\",\"message\":\"" + expectedError + "\"}"));
         assertThat(session.getStatus(), Is.is(Session.Status.PREPARE));
-        return activateRequest;
     }
 
     private void testUnsupportedMethod(com.yahoo.container.jdisc.HttpRequest request) throws Exception {
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandlerTest.java
index 310342e81f1..65b12490b17 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandlerTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandlerTest.java
@@ -45,7 +45,6 @@ import static org.junit.Assert.fail;
 
 /**
  * @author hmusum
- * @since 5.1
  */
 public class SessionCreateHandlerTest extends SessionHandlerTest {
 
@@ -106,7 +105,7 @@ public class SessionCreateHandlerTest extends SessionHandlerTest {
         assertThat(factory.applicationName, is("ulfio"));
     }
 
-    protected void assertFromParameter(String expected, String from) throws IOException {
+    private void assertFromParameter(String expected, String from) throws IOException {
         HttpRequest request = post(Collections.singletonMap("from", from));
         MockSessionFactory factory = new MockSessionFactory();
         factory.applicationPackage = testApp;
@@ -120,7 +119,7 @@ public class SessionCreateHandlerTest extends SessionHandlerTest {
                               expected + "/content/\",\"message\":\"Session " + expected + createdMessage + "}"));
     }
 
-    protected void assertIllegalFromParameter(String fromValue) throws IOException {
+    private void assertIllegalFromParameter(String fromValue) throws IOException {
         File outFile = CompressedApplicationInputStreamTest.createTarFile();
         HttpRequest request = post(outFile, postHeaders, Collections.singletonMap("from", fromValue));
         HandlerTest.assertHttpStatusCodeErrorCodeAndMessage(createHandler().handle(request), BAD_REQUEST, HttpErrorResponse.errorCodes.BAD_REQUEST, "Parameter 'from' has illegal value '" + fromValue + "'");
@@ -218,7 +217,7 @@ public class SessionCreateHandlerTest extends SessionHandlerTest {
         assertIllegalFromParameter("http://host:4013/application/v2/tenant/" + tenant + "/application/foo/environment/prod/region/baz/instance");
     }
 
-    public SessionCreateHandler createHandler() {
+    private SessionCreateHandler createHandler() {
         try {
             return createHandler(new MockSessionFactory());
         } catch (Exception e) {
@@ -228,7 +227,7 @@ public class SessionCreateHandlerTest extends SessionHandlerTest {
         return null;
     }
 
-    public SessionCreateHandler createHandler(SessionFactory sessionFactory) {
+    private SessionCreateHandler createHandler(SessionFactory sessionFactory) {
         try {
             TestTenantBuilder testBuilder = new TestTenantBuilder();
             testBuilder.createTenant(tenant).withSessionFactory(sessionFactory)
@@ -250,15 +249,15 @@ public class SessionCreateHandlerTest extends SessionHandlerTest {
                                                                   Clock.systemUTC()));
     }
 
-    public HttpRequest post() throws FileNotFoundException {
+    private HttpRequest post() throws FileNotFoundException {
         return post(null, postHeaders, new HashMap<>());
     }
 
-    public HttpRequest post(File file) throws FileNotFoundException {
+    private HttpRequest post(File file) throws FileNotFoundException {
         return post(file, postHeaders, new HashMap<>());
     }
 
-    public HttpRequest post(File file, Map<String, String> headers, Map<String, String> parameters) throws FileNotFoundException {
+    private HttpRequest post(File file, Map<String, String> headers, Map<String, String> parameters) throws FileNotFoundException {
         HttpRequest request = HttpRequest.createTestRequest("http://" + hostname + ":" + port + "/application/v2/tenant/" + tenant + "/session",
                 POST,
                 file == null ? null : new FileInputStream(file),
@@ -269,7 +268,7 @@ public class SessionCreateHandlerTest extends SessionHandlerTest {
         return request;
     }
 
-    public HttpRequest post(Map<String, String> parameters) throws FileNotFoundException {
+    private HttpRequest post(Map<String, String> parameters) throws FileNotFoundException {
         return post(null, new HashMap<>(), parameters);
     }
 }
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java
index 7900a67bddd..74a2dcf8054 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java
@@ -32,7 +32,6 @@ import com.yahoo.vespa.config.server.http.*;
 import com.yahoo.vespa.config.server.session.*;
 import com.yahoo.vespa.curator.Curator;
 import com.yahoo.vespa.curator.mock.MockCurator;
-import org.apache.commons.lang.NullArgumentException;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -148,7 +147,7 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest {
      * A mock remote session repo based on contents of local repo
      */
     private RemoteSessionRepo fromLocalSessionRepo(LocalSessionRepo localRepo, Clock clock) {
-        RemoteSessionRepo remoteRepo = new RemoteSessionRepo();
+        RemoteSessionRepo remoteRepo = new RemoteSessionRepo(tenant);
         for (LocalSession ls : localRepo.listSessions()) {
 
             zooKeeperClient = new MockSessionZKClient(curator, tenant, ls.getSessionId());
@@ -239,7 +238,7 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest {
         // Need different repos for 'default' tenant as opposed to the 'test' tenant
         LocalSessionRepo localRepoDefault = new LocalSessionRepo(Clock.systemUTC());
         final TenantName defaultTenant = TenantName.defaultName();
-        addTenant(defaultTenant, localRepoDefault, new RemoteSessionRepo(), new MockSessionFactory());
+        addTenant(defaultTenant, localRepoDefault, new RemoteSessionRepo(tenant), new MockSessionFactory());
         addTestTenant();
         final SessionHandler handler = createHandler(builder);
 
@@ -379,7 +378,7 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest {
     }
 
     private TestTenantBuilder addTestTenant() {
-        return addTenant(tenant, localRepo, new RemoteSessionRepo(), new MockSessionFactory());
+        return addTenant(tenant, localRepo, new RemoteSessionRepo(tenant), new MockSessionFactory());
     }
 
     private SessionHandler createHandler(TestTenantBuilder builder) {
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/TestTenantBuilder.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/TestTenantBuilder.java
index 16ce605d4d1..32f29858b74 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/TestTenantBuilder.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/TestTenantBuilder.java
@@ -19,7 +19,6 @@ import java.util.*;
  * Test utility for creating tenants used for testing and setup wiring of tenant stuff.
  *
  * @author Ulf Lilleengen
- * @since 5.1
  */
 public class TestTenantBuilder {
 
@@ -35,7 +34,7 @@ public class TestTenantBuilder {
         TenantBuilder builder = TenantBuilder.create(componentRegistry, tenantName)
                 .withSessionFactory(new SessionCreateHandlerTest.MockSessionFactory())
                 .withLocalSessionRepo(new LocalSessionRepo(componentRegistry.getClock()))
-                .withRemoteSessionRepo(new RemoteSessionRepo())
+                .withRemoteSessionRepo(new RemoteSessionRepo(tenantName))
                 .withApplicationRepo(applicationRepo);
         tenantMap.put(tenantName, builder);
         return builder;