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);