Put submission stuff in a separate class

7 files changed, 95 insertions, 57 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
index b0966f7db21..7ba8191d1fc 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
@@ -461,9 +461,7 @@ public class JobController {
     }
 
     /** Accepts and stores a new application package and test jar pair under a generated application version key. */
-    public ApplicationVersion submit(TenantAndApplicationId id, Optional<SourceRevision> revision, Optional<String> authorEmail,
-                                     Optional<String> sourceUrl, long projectId, ApplicationPackage applicationPackage,
-                                     byte[] testPackageBytes, Optional<String> description, int risk) {
+    public ApplicationVersion submit(TenantAndApplicationId id, Submission submission, long projectId) {
         ApplicationController applications = controller.applications();
         AtomicReference<ApplicationVersion> version = new AtomicReference<>();
         applications.lockApplicationOrThrow(id, application -> {
@@ -471,37 +469,26 @@ public class JobController {
             Optional<ApplicationPackage> previousPackage = previousVersion.flatMap(previous -> applications.applicationStore().find(id.tenant(), id.application(), previous.buildNumber().getAsLong()))
                                                                           .map(ApplicationPackage::new);
             long previousBuild = previousVersion.map(latestVersion -> latestVersion.buildNumber().getAsLong()).orElse(0L);
-            String packageHash = applicationPackage.bundleHash() + ApplicationPackage.calculateHash(testPackageBytes);
-            RevisionId revisionId = RevisionId.forProduction(1 + previousBuild);
-            version.set(ApplicationVersion.forProduction(revisionId,
-                                                         revision,
-                                                         authorEmail,
-                                                         applicationPackage.compileVersion(),
-                                                         applicationPackage.buildTime(),
-                                                         sourceUrl,
-                                                         revision.map(SourceRevision::commit),
-                                                         Optional.of(packageHash),
-                                                         description,
-                                                         risk));
-
-            byte[] diff = previousPackage.map(previous -> ApplicationPackageDiff.diff(previous, applicationPackage))
-                                         .orElseGet(() -> ApplicationPackageDiff.diffAgainstEmpty(applicationPackage));
+            version.set(submission.toApplicationVersion(1 + previousBuild));
+
+            byte[] diff = previousPackage.map(previous -> ApplicationPackageDiff.diff(previous, submission.applicationPackage()))
+                                         .orElseGet(() -> ApplicationPackageDiff.diffAgainstEmpty(submission.applicationPackage()));
             applications.applicationStore().put(id.tenant(),
                                                 id.application(),
                                                 version.get().id(),
-                                                applicationPackage.zippedContent(),
-                                                testPackageBytes,
+                                                submission.applicationPackage().zippedContent(),
+                                                submission.testPackage(),
                                                 diff);
             applications.applicationStore().putMeta(id.tenant(),
                                                     id.application(),
                                                     controller.clock().instant(),
-                                                    applicationPackage.metaDataZip());
+                                                    submission.applicationPackage().metaDataZip());
 
             application = application.withProjectId(projectId == -1 ? OptionalLong.empty() : OptionalLong.of(projectId));
             application = application.withRevisions(revisions -> revisions.with(version.get()));
             application = withPrunedPackages(application);
 
-            TestSummary testSummary = TestPackage.validateTests(applicationPackage.deploymentSpec(), testPackageBytes);
+            TestSummary testSummary = TestPackage.validateTests(submission.applicationPackage().deploymentSpec(), submission.testPackage());
             if (testSummary.problems().isEmpty())
                 controller.notificationsDb().removeNotification(NotificationSource.from(id),
                                                                 Type.testPackage);
@@ -511,7 +498,7 @@ public class JobController {
                                                              Notification.Level.warning,
                                                              testSummary.problems());
 
-            applications.storeWithUpdatedConfig(application, applicationPackage);
+            applications.storeWithUpdatedConfig(application, submission.applicationPackage());
             applications.deploymentTrigger().triggerNewRevision(id);
         });
         return version.get();
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Submission.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Submission.java
new file mode 100644
index 00000000000..feb15ce188b
--- /dev/null
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Submission.java
@@ -0,0 +1,59 @@
+package com.yahoo.vespa.hosted.controller.deployment;
+
+import com.yahoo.component.Version;
+import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion;
+import com.yahoo.vespa.hosted.controller.api.integration.deployment.RevisionId;
+import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision;
+import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackage;
+
+import java.time.Instant;
+import java.util.Optional;
+
+import static com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackage.calculateHash;
+
+/**
+ * @author jonmv
+ */
+public class Submission {
+
+    private final ApplicationPackage applicationPackage;
+    private final byte[] testPackage;
+    private final Optional<String> sourceUrl;
+    private final Optional<SourceRevision> source;
+    private final Optional<String> authorEmail;
+    private final Optional<String> description;
+    private final int risk;
+
+    public Submission(ApplicationPackage applicationPackage, byte[] testPackage, Optional<String> sourceUrl,
+                      Optional<SourceRevision> source, Optional<String> authorEmail, Optional<String> description, int risk) {
+        this.applicationPackage = applicationPackage;
+        this.testPackage = testPackage;
+        this.sourceUrl = sourceUrl;
+        this.source = source;
+        this.authorEmail = authorEmail;
+        this.description = description;
+        this.risk = risk;
+    }
+
+    public static Submission basic(ApplicationPackage applicationPackage, byte[] testPackage) {
+        return new Submission(applicationPackage, testPackage, Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), 0);
+    }
+
+    public ApplicationVersion toApplicationVersion(long number) {
+        return ApplicationVersion.forProduction(RevisionId.forProduction(number),
+                                                source,
+                                                authorEmail,
+                                                applicationPackage.compileVersion(),
+                                                applicationPackage.buildTime(),
+                                                sourceUrl,
+                                                source.map(SourceRevision::commit),
+                                                Optional.of(applicationPackage.bundleHash() + calculateHash(testPackage)),
+                                                description,
+                                                risk);
+    }
+
+    public ApplicationPackage applicationPackage() { return applicationPackage; }
+
+    public byte[] testPackage() { return testPackage; }
+
+}
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
index 6dc5e4565de..d163ba9d02c 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
@@ -94,6 +94,7 @@ import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger.ChangesToCancel;
 import com.yahoo.vespa.hosted.controller.deployment.JobStatus;
 import com.yahoo.vespa.hosted.controller.deployment.Run;
+import com.yahoo.vespa.hosted.controller.deployment.Submission;
 import com.yahoo.vespa.hosted.controller.deployment.TestConfigSerializer;
 import com.yahoo.vespa.hosted.controller.maintenance.ResourceMeterMaintainer;
 import com.yahoo.vespa.hosted.controller.notification.Notification;
@@ -2713,31 +2714,26 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
 
         ApplicationPackage applicationPackage = new ApplicationPackage(dataParts.get(EnvironmentResource.APPLICATION_ZIP), true);
 
+        byte[] testPackage = dataParts.get(EnvironmentResource.APPLICATION_TEST_ZIP);
+        Submission submission = new Submission(applicationPackage, testPackage, sourceUrl, sourceRevision, authorEmail, description, risk);
+
         controller.applications().verifyApplicationIdentityConfiguration(TenantName.from(tenant),
                                                                          Optional.empty(),
                                                                          Optional.empty(),
                                                                          applicationPackage,
                                                                          Optional.of(requireUserPrincipal(request)));
 
-        ensureApplicationExists(TenantAndApplicationId.from(tenant, application), request);
-
-        return JobControllerApiHandlerHelper.submitResponse(controller.jobController(),
-                                                            tenant,
-                                                            application,
-                                                            sourceRevision,
-                                                            authorEmail,
-                                                            sourceUrl,
-                                                            description,
-                                                            risk,
-                                                            projectId,
-                                                            applicationPackage,
-                                                            dataParts.get(EnvironmentResource.APPLICATION_TEST_ZIP));
+        TenantAndApplicationId id = TenantAndApplicationId.from(tenant, application);
+        ensureApplicationExists(id, request);
+        return JobControllerApiHandlerHelper.submitResponse(controller.jobController(), id, submission, projectId);
     }
 
     private HttpResponse removeAllProdDeployments(String tenant, String application) {
-        JobControllerApiHandlerHelper.submitResponse(controller.jobController(), tenant, application,
-                Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), 0, 1,
-                ApplicationPackage.deploymentRemoval(), new byte[0]);
+        JobControllerApiHandlerHelper.submitResponse(controller.jobController(),
+                                                     TenantAndApplicationId.from(tenant, application),
+                                                     new Submission(ApplicationPackage.deploymentRemoval(), new byte[0], Optional.empty(),
+                                                                    Optional.empty(), Optional.empty(), Optional.empty(), 0),
+                                                     0);
         return new MessageResponse("All deployments removed");
     }
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java
index de03bbfb767..80425609aa6 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java
@@ -34,6 +34,7 @@ import com.yahoo.vespa.hosted.controller.deployment.Run;
 import com.yahoo.vespa.hosted.controller.deployment.RunLog;
 import com.yahoo.vespa.hosted.controller.deployment.RunStatus;
 import com.yahoo.vespa.hosted.controller.deployment.Step;
+import com.yahoo.vespa.hosted.controller.deployment.Submission;
 import com.yahoo.vespa.hosted.controller.deployment.Versions;
 import com.yahoo.vespa.hosted.controller.versions.VersionStatus;
 import com.yahoo.vespa.hosted.controller.versions.VespaVersion;
@@ -194,14 +195,8 @@ class JobControllerApiHandlerHelper {
      *
      * @return Response with the new application version
      */
-    static HttpResponse submitResponse(JobController jobController, String tenant, String application,
-                                       Optional<SourceRevision> sourceRevision, Optional<String> authorEmail,
-                                       Optional<String> sourceUrl, Optional<String> description, int risk, long projectId,
-                                       ApplicationPackage applicationPackage, byte[] testPackage) {
-        ApplicationVersion version = jobController.submit(TenantAndApplicationId.from(tenant, application), sourceRevision, authorEmail,
-                                                          sourceUrl, projectId, applicationPackage, testPackage, description, risk);
-
-        return new MessageResponse("application " + version);
+    static HttpResponse submitResponse(JobController jobController, TenantAndApplicationId id, Submission submission, long projectId) {
+        return new MessageResponse("application " + jobController.submit(id, submission, projectId));
     }
 
     /** Aborts any job of the given type. */
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
index 20b64419f28..0ecac036913 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
@@ -38,6 +38,7 @@ import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackage;
 import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentContext;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester;
+import com.yahoo.vespa.hosted.controller.deployment.Submission;
 import com.yahoo.vespa.hosted.controller.integration.ZoneApiMock;
 import com.yahoo.vespa.hosted.controller.notification.Notification;
 import com.yahoo.vespa.hosted.controller.notification.Notification.Level;
@@ -1118,8 +1119,7 @@ public class ControllerTest {
         ApplicationPackage applicationPackage = ApplicationPackageBuilder.fromDeploymentXml(deploymentXml);
         byte[] testPackage = ApplicationPackage.filesZip(Map.of("tests/staging-test/foo.json", new byte[0]));
         var app = tester.newDeploymentContext();
-        tester.jobs().submit(app.application().id(), Optional.empty(), Optional.empty(), Optional.empty(), 1,
-                             applicationPackage, testPackage, Optional.empty(), 0);
+        tester.jobs().submit(app.application().id(), Submission.basic(applicationPackage, testPackage), 1);
         assertEquals(List.of(new Notification(tester.clock().instant(),
                                               Type.testPackage,
                                               Level.warning,
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java
index ad6a987d42b..fd294f9cf9f 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java
@@ -311,7 +311,7 @@ public class DeploymentContext {
                               .projectId()
                               .orElse(1000); // These are really set through submission, so just pick one if it hasn't been set.
         var testerpackage = new byte[]{ (byte) (salt >> 56), (byte) (salt >> 48), (byte) (salt >> 40), (byte) (salt >> 32), (byte) (salt >> 24), (byte) (salt >> 16), (byte) (salt >> 8), (byte) salt };
-        lastSubmission = jobs.submit(applicationId, sourceRevision, Optional.of("a@b"), Optional.empty(), projectId, applicationPackage, testerpackage, Optional.empty(), risk).id();
+        lastSubmission = jobs.submit(applicationId, new Submission(applicationPackage, testerpackage, Optional.empty(), sourceRevision, Optional.of("a@b"), Optional.empty(), risk), projectId).id();
         return this;
     }
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java
index 19971a0ee5b..5fd1e8347ef 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java
@@ -18,6 +18,7 @@ import com.yahoo.vespa.hosted.controller.deployment.RunStatus;
 import com.yahoo.vespa.hosted.controller.deployment.Step;
 import com.yahoo.vespa.hosted.controller.deployment.Step.Status;
 import com.yahoo.vespa.hosted.controller.deployment.StepRunner;
+import com.yahoo.vespa.hosted.controller.deployment.Submission;
 import com.yahoo.vespa.hosted.controller.deployment.Versions;
 import com.yahoo.vespa.hosted.controller.integration.MetricsMock;
 import org.junit.Test;
@@ -91,9 +92,9 @@ public class JobRunnerTest {
         TenantAndApplicationId appId = tester.createApplication("tenant", "real", "default").id();
         ApplicationId id = appId.defaultInstance();
         byte[] testPackageBytes = new byte[0];
-        jobs.submit(appId, Optional.empty(), Optional.empty(), Optional.empty(), 2, applicationPackage, testPackageBytes, Optional.empty(), 0);
+        jobs.submit(appId, Submission.basic(applicationPackage, testPackageBytes), 2);
 
-        start(jobs, id, systemTest);
+                    start(jobs, id, systemTest);
         try {
             start(jobs, id, systemTest);
             fail("Job is already running, so this should not be allowed!");
@@ -123,7 +124,7 @@ public class JobRunnerTest {
         TenantAndApplicationId appId = tester.createApplication("tenant", "real", "default").id();
         ApplicationId id = appId.defaultInstance();
         byte[] testPackageBytes = new byte[0];
-        jobs.submit(appId, Optional.empty(), Optional.empty(), Optional.empty(), 2, applicationPackage, testPackageBytes, Optional.empty(), 0);
+        jobs.submit(appId, Submission.basic(applicationPackage, testPackageBytes), 2);
         Supplier<Run> run = () -> jobs.last(id, systemTest).get();
 
         start(jobs, id, systemTest);
@@ -231,7 +232,7 @@ public class JobRunnerTest {
         TenantAndApplicationId appId = tester.createApplication("tenant", "real", "default").id();
         ApplicationId id = appId.defaultInstance();
         byte[] testPackageBytes = new byte[0];
-        jobs.submit(appId, Optional.empty(), Optional.empty(), Optional.empty(), 2, applicationPackage, testPackageBytes, Optional.empty(), 0);
+        jobs.submit(appId, Submission.basic(applicationPackage, testPackageBytes), 2);
 
         RunId runId = new RunId(id, systemTest, 1);
         start(jobs, id, systemTest);
@@ -269,7 +270,7 @@ public class JobRunnerTest {
         ApplicationId instanceId = appId.defaultInstance();
         JobId jobId = new JobId(instanceId, systemTest);
         byte[] testPackageBytes = new byte[0];
-        jobs.submit(appId, Optional.empty(), Optional.empty(), Optional.empty(), 2, applicationPackage, testPackageBytes, Optional.empty(), 0);
+        jobs.submit(appId, Submission.basic(applicationPackage, testPackageBytes), 2);
         assertFalse(jobs.lastSuccess(jobId).isPresent());
 
         for (int i = 0; i < jobs.historyLength(); i++) {
@@ -365,7 +366,7 @@ public class JobRunnerTest {
         TenantAndApplicationId appId = tester.createApplication("tenant", "real", "default").id();
         ApplicationId id = appId.defaultInstance();
         byte[] testPackageBytes = new byte[0];
-        jobs.submit(appId, Optional.empty(), Optional.empty(), Optional.empty(), 2, applicationPackage, testPackageBytes, Optional.empty(), 0);
+        jobs.submit(appId, Submission.basic(applicationPackage, testPackageBytes), 2);
 
         start(jobs, id, systemTest);
         tester.clock().advance(JobRunner.jobTimeout.plus(Duration.ofSeconds(1)));
@@ -383,7 +384,7 @@ public class JobRunnerTest {
         TenantAndApplicationId appId = tester.createApplication("tenant", "real", "default").id();
         ApplicationId id = appId.defaultInstance();
         byte[] testPackageBytes = new byte[0];
-        jobs.submit(appId, Optional.empty(), Optional.empty(), Optional.empty(), 2, applicationPackage, testPackageBytes, Optional.empty(), 0);
+        jobs.submit(appId, Submission.basic(applicationPackage, testPackageBytes), 2);
 
         for (Step step : JobProfile.of(systemTest).steps())
             outcomes.put(step, running);