Make ApplicationVersion id depend on number and commit, and make revision optional

5 files changed, 23 insertions, 21 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 e998e4c8ef9..811daed256e 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
@@ -372,7 +372,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, SourceRevision revision, String authorEmail,
+    public ApplicationVersion submit(TenantAndApplicationId id, Optional<SourceRevision> revision, Optional<String> authorEmail,
                                      Optional<String> sourceUrl, Optional<String> commit,
                                      long projectId, ApplicationPackage applicationPackage, byte[] testPackageBytes) {
         AtomicReference<ApplicationVersion> version = new AtomicReference<>();
@@ -380,14 +380,11 @@ public class JobController {
             long run = 1 + application.get().latestVersion()
                                       .map(latestVersion -> latestVersion.buildNumber().getAsLong())
                                       .orElse(0L);
-            if (applicationPackage.compileVersion().isPresent() && applicationPackage.buildTime().isPresent())
-                version.set(ApplicationVersion.from(revision, run, authorEmail,
-                                                    applicationPackage.compileVersion().get(),
-                                                    applicationPackage.buildTime().get(),
-                                                    sourceUrl,
-                                                    commit));
-            else
-                version.set(ApplicationVersion.from(revision, run, authorEmail));
+            version.set(ApplicationVersion.from(revision, run, authorEmail,
+                                                applicationPackage.compileVersion(),
+                                                applicationPackage.buildTime(),
+                                                sourceUrl,
+                                                commit));
 
             controller.applications().applicationStore().put(id.tenant(),
                                                              id.application(),
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 e92c52a613f..0c0b1f67b90 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
@@ -1988,11 +1988,15 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
     private HttpResponse submit(String tenant, String application, HttpRequest request) {
         Map<String, byte[]> dataParts = parseDataParts(request);
         Inspector submitOptions = SlimeUtils.jsonToSlime(dataParts.get(EnvironmentResource.SUBMIT_OPTIONS)).get();
-        SourceRevision sourceRevision = toSourceRevision(submitOptions);
-        String authorEmail = submitOptions.field("authorEmail").asString();
         long projectId = Math.max(1, submitOptions.field("projectId").asLong());
-        Optional<String> commit = submitOptions.field("commit").valid() ? Optional.of(submitOptions.field("commit").asString()) : Optional.empty();
-        Optional<String> sourceUrl = submitOptions.field("sourceUrl").valid() ? Optional.of(submitOptions.field("sourceUrl").asString()) : Optional.empty();
+        Optional<String> repository = optional("repository", submitOptions);
+        Optional<String> branch = optional("branch", submitOptions);
+        Optional<String> commit = optional("commit", submitOptions);
+        Optional<SourceRevision> sourceRevision = repository.isPresent() && branch.isPresent() && commit.isPresent()
+                                                  ? Optional.of(new SourceRevision(repository.get(), branch.get(), commit.get()))
+                                                  : Optional.empty();
+        Optional<String> sourceUrl = optional("sourceUrl", submitOptions);
+        Optional<String> authorEmail = optional("authorEmail", submitOptions);
 
         ApplicationPackage applicationPackage = new ApplicationPackage(dataParts.get(EnvironmentResource.APPLICATION_ZIP), true);
 
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 4bcf744ead5..8cc309c33aa 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
@@ -472,8 +472,8 @@ class JobControllerApiHandlerHelper {
      * @return Response with the new application version
      */
     static HttpResponse submitResponse(JobController jobController, String tenant, String application,
-                                       SourceRevision sourceRevision, String authorEmail, Optional<String> sourceUrl,
-                                       Optional<String> commit, long projectId,
+                                       Optional<SourceRevision> sourceRevision, Optional<String> authorEmail,
+                                       Optional<String> sourceUrl, Optional<String> commit, long projectId,
                                        ApplicationPackage applicationPackage, byte[] testPackage) {
         ApplicationVersion version = jobController.submit(TenantAndApplicationId.from(tenant, application),
                                                           sourceRevision,
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 bd3dc5d4336..2d0b625dcb3 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
@@ -208,7 +208,8 @@ public class DeploymentContext {
                               .requireApplication(applicationId)
                               .projectId()
                               .orElse(1000); // These are really set through submission, so just pick one if it hasn't been set.
-        lastSubmission = jobs.submit(applicationId, sourceRevision, "a@b", Optional.empty(), Optional.empty(), projectId, applicationPackage, new byte[0]);
+        lastSubmission = jobs.submit(applicationId, Optional.of(sourceRevision), Optional.of("a@b"), Optional.empty(),
+                                     Optional.empty(), projectId, applicationPackage, new byte[0]);
         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 b2ec7ec5f26..21e4735f7bf 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
@@ -87,7 +87,7 @@ public class JobRunnerTest {
 
         TenantAndApplicationId appId = tester.createApplication("tenant", "real", "default").id();
         ApplicationId id = appId.defaultInstance();
-        jobs.submit(appId, versions.targetApplication().source().get(), "a@b", Optional.empty(), Optional.empty(), 2, applicationPackage, new byte[0]);
+        jobs.submit(appId, versions.targetApplication().source(), Optional.empty(), Optional.empty(), Optional.empty(), 2, applicationPackage, new byte[0]);
 
         jobs.start(id, systemTest, versions);
         try {
@@ -119,7 +119,7 @@ public class JobRunnerTest {
 
         TenantAndApplicationId appId = tester.createApplication("tenant", "real", "default").id();
         ApplicationId id = appId.defaultInstance();
-        jobs.submit(appId, versions.targetApplication().source().get(), "a@b", Optional.empty(), Optional.empty(), 2, applicationPackage, new byte[0]);
+        jobs.submit(appId, versions.targetApplication().source(), Optional.empty(), Optional.empty(), Optional.empty(), 2, applicationPackage, new byte[0]);
         Supplier<Run> run = () -> jobs.last(id, systemTest).get();
 
         jobs.start(id, systemTest, versions);
@@ -227,7 +227,7 @@ public class JobRunnerTest {
 
         TenantAndApplicationId appId = tester.createApplication("tenant", "real", "default").id();
         ApplicationId id = appId.defaultInstance();
-        jobs.submit(appId, versions.targetApplication().source().get(), "a@b", Optional.empty(), Optional.empty(), 2, applicationPackage, new byte[0]);
+        jobs.submit(appId, versions.targetApplication().source(), Optional.empty(), Optional.empty(), Optional.empty(), 2, applicationPackage, new byte[0]);
 
         RunId runId = new RunId(id, systemTest, 1);
         jobs.start(id, systemTest, versions);
@@ -265,7 +265,7 @@ public class JobRunnerTest {
         TenantAndApplicationId appId = tester.createApplication("tenant", "real", "default").id();
         ApplicationId instanceId = appId.defaultInstance();
         JobId jobId = new JobId(instanceId, systemTest);
-        jobs.submit(appId, versions.targetApplication().source().get(), "a@b", Optional.empty(), Optional.empty(), 2, applicationPackage, new byte[0]);
+        jobs.submit(appId, versions.targetApplication().source(), Optional.empty(), Optional.empty(), Optional.empty(), 2, applicationPackage, new byte[0]);
         assertFalse(jobs.lastSuccess(jobId).isPresent());
 
         for (int i = 0; i < jobs.historyLength(); i++) {
@@ -343,7 +343,7 @@ public class JobRunnerTest {
 
         TenantAndApplicationId appId = tester.createApplication("tenant", "real", "default").id();
         ApplicationId id = appId.defaultInstance();
-        jobs.submit(appId, versions.targetApplication().source().get(), "a@b", Optional.empty(), Optional.empty(), 2, applicationPackage, new byte[0]);
+        jobs.submit(appId, versions.targetApplication().source(), Optional.empty(), Optional.empty(), Optional.empty(), 2, applicationPackage, new byte[0]);
 
         jobs.start(id, systemTest, versions);
         tester.clock().advance(JobRunner.jobTimeout.plus(Duration.ofSeconds(1)));