diff options
author | Harald Musum <musum@yahoo-inc.com> | 2016-12-05 14:18:03 +0100 |
---|---|---|
committer | Harald Musum <musum@yahoo-inc.com> | 2016-12-05 14:18:03 +0100 |
commit | a01c5af50bef58108f907e9c3a812b9b32d792c1 (patch) | |
tree | dd07646e200cb6fd6d455cebe634c405a3861bbb | |
parent | db3cb827b5e95ea9b504b364d566a1bbe79fcfc8 (diff) |
Add builder for PrepareParams and make it immutable.
5 files changed, 107 insertions, 77 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 9af2750b015..f934c1e3f57 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 @@ -103,7 +103,7 @@ public class Deployment implements com.yahoo.config.provision.Deployment { TimeoutBudget timeoutBudget = new TimeoutBudget(clock, timeout); session.prepare(logger, /** Assumes that session has already set application id, see {@link com.yahoo.vespa.config.server.session.SessionFactoryImpl}. */ - new PrepareParams(configserverConfig).applicationId(session.getApplicationId()).timeoutBudget(timeoutBudget).ignoreValidationErrors( ! validate), + new PrepareParams.Builder().applicationId(session.getApplicationId()).timeoutBudget(timeoutBudget).ignoreValidationErrors( ! validate).build(), Optional.empty(), tenantPath); this.prepared = true; 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 6c9224db0fe..8ed80ac2339 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 @@ -17,7 +17,7 @@ import java.util.Optional; import java.util.Set; /** - * Parameters for prepare. + * Parameters for prepare. Immutable. * * @author lulf * @since 5.1.24 @@ -32,79 +32,102 @@ public final class PrepareParams { static final String ROTATIONS_PARAM_NAME = "rotations"; static final String DOCKER_VESPA_IMAGE_VERSION_PARAM_NAME = "dockerVespaImageVersion"; - private boolean ignoreValidationErrors = false; - private boolean dryRun = false; - private ApplicationId applicationId = ApplicationId.defaultId(); - private TimeoutBudget timeoutBudget; - private Optional<Version> vespaVersion = Optional.empty(); - private Set<Rotation> rotations; - private Optional<Version> dockerVespaImageVersion = Optional.empty(); - - PrepareParams() { - this(new ConfigserverConfig(new ConfigserverConfig.Builder())); + private final ApplicationId applicationId; + private final TimeoutBudget timeoutBudget; + private final boolean ignoreValidationErrors; + private final boolean dryRun; + private final Optional<Version> vespaVersion; + private final Set<Rotation> rotations; + private final Optional<Version> dockerVespaImageVersion; + + public PrepareParams(ApplicationId applicationId, TimeoutBudget timeoutBudget, boolean ignoreValidationErrors, + boolean dryRun, Optional<Version> vespaVersion, Set<Rotation> rotations, + Optional<Version> dockerVespaImageVersion) { + this.timeoutBudget = timeoutBudget; + this.applicationId = applicationId; + this.ignoreValidationErrors = ignoreValidationErrors; + this.dryRun = dryRun; + this.vespaVersion = vespaVersion; + this.rotations = rotations; + this.dockerVespaImageVersion = dockerVespaImageVersion; } - public PrepareParams(ConfigserverConfig configserverConfig) { - timeoutBudget = new TimeoutBudget(Clock.systemUTC(), getBarrierTimeout(configserverConfig)); - } + public static class Builder { + private boolean ignoreValidationErrors = false; + private boolean dryRun = false; + private ApplicationId applicationId = ApplicationId.defaultId(); + private TimeoutBudget timeoutBudget = new TimeoutBudget(Clock.systemUTC(), Duration.ofSeconds(5)); + private Optional<Version> vespaVersion = Optional.empty(); + private Set<Rotation> rotations; + private Optional<Version> dockerVespaImageVersion = Optional.empty(); - public PrepareParams applicationId(ApplicationId applicationId) { - this.applicationId = applicationId; - return this; - } + public Builder() { } - public PrepareParams ignoreValidationErrors(boolean ignoreValidationErrors) { - this.ignoreValidationErrors = ignoreValidationErrors; - return this; - } + public Builder applicationId(ApplicationId applicationId) { + this.applicationId = applicationId; + return this; + } - public PrepareParams dryRun(boolean dryRun) { - this.dryRun = dryRun; - return this; - } + public Builder ignoreValidationErrors(boolean ignoreValidationErrors) { + this.ignoreValidationErrors = ignoreValidationErrors; + return this; + } - public PrepareParams timeoutBudget(TimeoutBudget timeoutBudget) { - this.timeoutBudget = timeoutBudget; - return this; - } + public Builder dryRun(boolean dryRun) { + this.dryRun = dryRun; + return this; + } - public PrepareParams vespaVersion(String vespaVersion) { - Optional<Version> version = Optional.empty(); - if (vespaVersion != null && !vespaVersion.isEmpty()) { - version = Optional.of(Version.fromString(vespaVersion)); + public Builder timeoutBudget(TimeoutBudget timeoutBudget) { + this.timeoutBudget = timeoutBudget; + return this; } - this.vespaVersion = version; - return this; - } - public PrepareParams rotations(String rotationsString) { - this.rotations = new LinkedHashSet<>(); - if (rotationsString != null && !rotationsString.isEmpty()) { - String[] rotations = rotationsString.split(","); - for (String s : rotations) { - this.rotations.add(new Rotation(s)); + public Builder vespaVersion(String vespaVersion) { + Optional<Version> version = Optional.empty(); + if (vespaVersion != null && !vespaVersion.isEmpty()) { + version = Optional.of(Version.fromString(vespaVersion)); } + this.vespaVersion = version; + return this; } - return this; - } - public PrepareParams dockerVespaImageVersion(String dockerVespaImageVersion) { - Optional<Version> version = Optional.empty(); - if (dockerVespaImageVersion != null && !dockerVespaImageVersion.isEmpty()) { - version = Optional.of(Version.fromString(dockerVespaImageVersion)); + public Builder rotations(String rotationsString) { + this.rotations = new LinkedHashSet<>(); + if (rotationsString != null && !rotationsString.isEmpty()) { + String[] rotations = rotationsString.split(","); + for (String s : rotations) { + this.rotations.add(new Rotation(s)); + } + } + return this; } - this.dockerVespaImageVersion = version; - return this; + + public Builder dockerVespaImageVersion(String dockerVespaImageVersion) { + Optional<Version> version = Optional.empty(); + if (dockerVespaImageVersion != null && !dockerVespaImageVersion.isEmpty()) { + version = Optional.of(Version.fromString(dockerVespaImageVersion)); + } + this.dockerVespaImageVersion = version; + return this; + } + + public PrepareParams build() { + return new PrepareParams(applicationId, timeoutBudget, ignoreValidationErrors, dryRun, + vespaVersion, rotations, dockerVespaImageVersion); + } + } public static PrepareParams fromHttpRequest(HttpRequest request, TenantName tenant, ConfigserverConfig configserverConfig) { - return new PrepareParams(configserverConfig).ignoreValidationErrors(request.getBooleanProperty(IGNORE_VALIDATION_PARAM_NAME)) - .dryRun(request.getBooleanProperty(DRY_RUN_PARAM_NAME)) - .timeoutBudget(SessionHandler.getTimeoutBudget(request, getBarrierTimeout(configserverConfig))) - .applicationId(createApplicationId(request, tenant)) - .vespaVersion(request.getProperty(VESPA_VERSION_PARAM_NAME)) - .rotations(request.getProperty(ROTATIONS_PARAM_NAME)) - .dockerVespaImageVersion(request.getProperty(DOCKER_VESPA_IMAGE_VERSION_PARAM_NAME)); + return new PrepareParams.Builder().ignoreValidationErrors(request.getBooleanProperty(IGNORE_VALIDATION_PARAM_NAME)) + .dryRun(request.getBooleanProperty(DRY_RUN_PARAM_NAME)) + .timeoutBudget(SessionHandler.getTimeoutBudget(request, getBarrierTimeout(configserverConfig))) + .applicationId(createApplicationId(request, tenant)) + .vespaVersion(request.getProperty(VESPA_VERSION_PARAM_NAME)) + .rotations(request.getProperty(ROTATIONS_PARAM_NAME)) + .dockerVespaImageVersion(request.getProperty(DOCKER_VESPA_IMAGE_VERSION_PARAM_NAME)) + .build(); } private static Duration getBarrierTimeout(ConfigserverConfig configserverConfig) { diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java index 887a931c900..caaf90a0f04 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java @@ -96,7 +96,7 @@ public class DeployTester { LocalSession session = tenant.getSessionFactory().createSession(testApp, appName, new SilentDeployLogger(), new TimeoutBudget(Clock.systemUTC(), Duration.ofSeconds(60))); ApplicationId id = ApplicationId.from(tenant.getName(), ApplicationName.from(appName), InstanceName.defaultName()); session.prepare(new SilentDeployLogger(), - new PrepareParams(new ConfigserverConfig(new ConfigserverConfig.Builder())).applicationId(id), + new PrepareParams.Builder().applicationId(id).build(), Optional.empty(), tenant.getPath()); session.createActivateTransaction().commit(); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionTest.java index c793945a738..6e3b9625987 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionTest.java @@ -130,7 +130,7 @@ public class LocalSessionTest { ApplicationId origId = new ApplicationId.Builder() .tenant("tenant") .applicationName("foo").instanceName("quux").build(); - doPrepare(session, new PrepareParams().applicationId(origId)); + doPrepare(session, new PrepareParams.Builder().applicationId(origId).build()); ProvisionInfo info = session.getProvisionInfo(); assertNotNull(info); assertThat(info.getHosts().size(), is(1)); @@ -140,7 +140,7 @@ public class LocalSessionTest { @Test public void require_that_application_metadata_is_correct() throws Exception { LocalSession session = createSession(TenantName.defaultName(), 3); - doPrepare(session, new PrepareParams()); + doPrepare(session, new PrepareParams.Builder().build()); assertThat(session.getMetaData().toString(), is("n/a, n/a, 0, 0, , 0")); } @@ -168,7 +168,7 @@ public class LocalSessionTest { } private void doPrepare(LocalSession session) { - doPrepare(session, new PrepareParams()); + doPrepare(session, new PrepareParams.Builder().build()); } private void doPrepare(LocalSession session, PrepareParams params) { diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionPreparerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionPreparerTest.java index 61b16a718fa..12cb5749daa 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionPreparerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionPreparerTest.java @@ -99,24 +99,30 @@ public class SessionPreparerTest extends TestWithCurator { @Test(expected = InvalidApplicationException.class) public void require_that_application_validation_exception_is_not_caught() throws IOException, SAXException { FilesApplicationPackage app = getApplicationPackage(invalidTestApp); - preparer.prepare(getContext(app), getLogger(), new PrepareParams(), Optional.empty(), tenantPath); + preparer.prepare(getContext(app), getLogger(), new PrepareParams.Builder().build(), Optional.empty(), tenantPath); } @Test public void require_that_application_validation_exception_is_ignored_if_forced() throws IOException, SAXException { FilesApplicationPackage app = getApplicationPackage(invalidTestApp); - preparer.prepare(getContext(app), getLogger(), new PrepareParams().ignoreValidationErrors(true).timeoutBudget(TimeoutBudgetTest.day()), Optional.empty(), tenantPath); + preparer.prepare(getContext(app), getLogger(), + new PrepareParams.Builder().ignoreValidationErrors(true).timeoutBudget(TimeoutBudgetTest.day()).build(), + Optional.empty(), tenantPath); } @Test public void require_that_zookeeper_is_not_written_to_if_dryrun() throws IOException { - preparer.prepare(getContext(getApplicationPackage(testApp)), getLogger(), new PrepareParams().dryRun(true).timeoutBudget(TimeoutBudgetTest.day()), Optional.empty(), tenantPath); + preparer.prepare(getContext(getApplicationPackage(testApp)), getLogger(), + new PrepareParams.Builder().dryRun(true).timeoutBudget(TimeoutBudgetTest.day()).build(), + Optional.empty(), tenantPath); assertFalse(configCurator.exists(appPath.append(ConfigCurator.USERAPP_ZK_SUBPATH).append("services.xml").getAbsolute())); } @Test public void require_that_filedistribution_is_ignored_on_dryrun() throws IOException { - preparer.prepare(getContext(getApplicationPackage(testApp)), getLogger(), new PrepareParams().dryRun(true).timeoutBudget(TimeoutBudgetTest.day()), Optional.empty(), tenantPath); + preparer.prepare(getContext(getApplicationPackage(testApp)), getLogger(), + new PrepareParams.Builder().dryRun(true).timeoutBudget(TimeoutBudgetTest.day()).build(), + Optional.empty(), tenantPath); assertThat(fileDistributionFactory.mockFileDistributionProvider.getMockFileDBHandler().sendDeployedFilesCalled, is(0)); assertThat(fileDistributionFactory.mockFileDistributionProvider.getMockFileDBHandler().limitSendingOfDeployedFilesToCalled, is(0)); assertThat(fileDistributionFactory.mockFileDistributionProvider.getMockFileDBHandler().reloadDeployFileDistributorCalled, is(0)); @@ -124,7 +130,7 @@ public class SessionPreparerTest extends TestWithCurator { @Test public void require_that_application_is_prepared() throws Exception { - preparer.prepare(getContext(getApplicationPackage(testApp)), getLogger(), new PrepareParams(), Optional.empty(), tenantPath); + preparer.prepare(getContext(getApplicationPackage(testApp)), getLogger(), new PrepareParams.Builder().build(), Optional.empty(), tenantPath); assertThat(fileDistributionFactory.mockFileDistributionProvider.getMockFileDBHandler().sendDeployedFilesCalled, is(2)); assertThat(fileDistributionFactory.mockFileDistributionProvider.getMockFileDBHandler().limitSendingOfDeployedFilesToCalled, is(2)); // Should be called only once no matter how many model versions are built @@ -138,7 +144,7 @@ public class SessionPreparerTest extends TestWithCurator { new TestModelFactory(Version.fromIntValues(1, 2, 3)), new FailingModelFactory(Version.fromIntValues(3, 2, 1), new IllegalArgumentException("BOOHOO")))); preparer = createPreparer(modelFactoryRegistry, HostProvisionerProvider.empty()); - preparer.prepare(getContext(getApplicationPackage(testApp)), getLogger(), new PrepareParams(), Optional.empty(), tenantPath); + preparer.prepare(getContext(getApplicationPackage(testApp)), getLogger(), new PrepareParams.Builder().build(), Optional.empty(), tenantPath); } @Test(expected = InvalidApplicationException.class) @@ -147,14 +153,14 @@ public class SessionPreparerTest extends TestWithCurator { new TestModelFactory(Version.fromIntValues(3, 2, 3)), new FailingModelFactory(Version.fromIntValues(1, 2, 1), new IllegalArgumentException("BOOHOO")))); preparer = createPreparer(modelFactoryRegistry, HostProvisionerProvider.empty()); - preparer.prepare(getContext(getApplicationPackage(testApp)), getLogger(), new PrepareParams(), Optional.empty(), tenantPath); + preparer.prepare(getContext(getApplicationPackage(testApp)), getLogger(), new PrepareParams.Builder().build(), Optional.empty(), tenantPath); } @Test(expected = InvalidApplicationException.class) public void require_exception_for_overlapping_host() throws IOException { SessionContext ctx = getContext(getApplicationPackage(testApp)); ((HostRegistry<ApplicationId>)ctx.getHostValidator()).update(applicationId("foo"), Collections.singletonList("mytesthost")); - preparer.prepare(ctx, new BaseDeployLogger(), new PrepareParams(), Optional.empty(), tenantPath); + preparer.prepare(ctx, new BaseDeployLogger(), new PrepareParams.Builder().build(), Optional.empty(), tenantPath); } @Test @@ -166,7 +172,7 @@ public class SessionPreparerTest extends TestWithCurator { System.out.println(level + ": "+message); if (level.equals(LogLevel.WARNING) && message.contains("The host mytesthost is already in use")) logged.append("ok"); }; - preparer.prepare(ctx, logger, new PrepareParams(), Optional.empty(), tenantPath); + preparer.prepare(ctx, logger, new PrepareParams.Builder().build(), Optional.empty(), tenantPath); assertEquals(logged.toString(), ""); } @@ -176,7 +182,7 @@ public class SessionPreparerTest extends TestWithCurator { ApplicationId origId = new ApplicationId.Builder() .tenant(tenant) .applicationName("foo").instanceName("quux").build(); - PrepareParams params = new PrepareParams().applicationId(origId); + PrepareParams params = new PrepareParams.Builder().applicationId(origId).build(); preparer.prepare(getContext(getApplicationPackage(testApp)), getLogger(), params, Optional.empty(), tenantPath); SessionZooKeeperClient zkc = new SessionZooKeeperClient(curator, appPath); assertTrue(configCurator.exists(appPath.append(SessionZooKeeperClient.APPLICATION_ID_PATH).getAbsolute())); @@ -193,8 +199,9 @@ public class SessionPreparerTest extends TestWithCurator { new MockRestartAction("other change", Arrays.asList(service))))); preparer = createPreparer(modelFactoryRegistry, HostProvisionerProvider.empty()); List<RestartActions.Entry> actions = - preparer.prepare(getContext(getApplicationPackage(testApp)), getLogger(), new PrepareParams(), Optional.empty(), tenantPath). - getRestartActions().getEntries(); + preparer.prepare(getContext(getApplicationPackage(testApp)), getLogger(), + new PrepareParams.Builder().build(), Optional.empty(), tenantPath) + .getRestartActions().getEntries(); assertThat(actions.size(), is(1)); assertThat(actions.get(0).getMessages(), equalTo(ImmutableSet.of("change", "other change"))); } @@ -207,7 +214,7 @@ public class SessionPreparerTest extends TestWithCurator { public void require_that_rotations_are_written_in_prepare() throws IOException { final String rotations = "mediasearch.msbe.global.vespa.yahooapis.com"; final ApplicationId applicationId = applicationId("test"); - PrepareParams params = new PrepareParams().applicationId(applicationId).rotations(rotations); + PrepareParams params = new PrepareParams.Builder().applicationId(applicationId).rotations(rotations).build(); File app = new File("src/test/resources/deploy/app"); preparer.prepare(getContext(getApplicationPackage(app)), getLogger(), params, Optional.empty(), tenantPath); assertThat(readRotationsFromZK(applicationId), contains(new Rotation(rotations))); @@ -223,7 +230,7 @@ public class SessionPreparerTest extends TestWithCurator { final String rotations = "foo.msbe.global.vespa.yahooapis.com"; final ApplicationId applicationId = applicationId("test"); new Rotations(curator, tenantPath).writeRotationsToZooKeeper(applicationId, Collections.singleton(new Rotation(rotations))); - final PrepareParams params = new PrepareParams().applicationId(applicationId); + final PrepareParams params = new PrepareParams.Builder().applicationId(applicationId).build(); final File app = new File("src/test/resources/deploy/app"); preparer.prepare(getContext(getApplicationPackage(app)), getLogger(), params, Optional.empty(), tenantPath); |