From 9d94f97af8c026ed1e647a9eb0acd55a0e4be8c5 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Wed, 29 Nov 2017 08:34:18 +0100 Subject: Set sessions path correctly in test constructor No functional changes, some other minor refactorings --- .../config/server/session/RemoteSessionRepo.java | 29 +++++++++++----------- .../server/http/v2/SessionActiveHandlerTest.java | 19 +++++++------- .../server/http/v2/SessionCreateHandlerTest.java | 17 ++++++------- .../server/http/v2/SessionPrepareHandlerTest.java | 7 +++--- .../config/server/http/v2/TestTenantBuilder.java | 3 +-- 5 files changed, 36 insertions(+), 39 deletions(-) (limited to 'configserver') 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 implements NodeCacheListener, PathChildrenCacheListener { @@ -74,6 +73,17 @@ public class RemoteSessionRepo extends SessionRepo 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 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 getSessionList(List children) { List 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 headers, Map parameters) throws FileNotFoundException { + private HttpRequest post(File file, Map headers, Map 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 parameters) throws FileNotFoundException { + private HttpRequest post(Map 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; -- cgit v1.2.3