More nitpicking

5 files changed, 28 insertions, 36 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java
index 1c5af1a8f2f..cfc272368cf 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java
@@ -13,7 +13,6 @@ import com.yahoo.vespa.hosted.controller.application.Change.VersionChange;
 import com.yahoo.vespa.hosted.controller.application.Deployment;
 import com.yahoo.vespa.hosted.controller.application.DeploymentJobs;
 
-import java.time.Instant;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
@@ -97,10 +96,9 @@ public class Application {
      * (deployments also includes manually deployed environments)
      */
     public Map<Zone, Deployment> productionDeployments() {
-        return deployments.values().stream()
-                                   .filter(deployment -> deployment.zone().environment() == Environment.prod)
-                                   .collect(Collectors.collectingAndThen(Collectors.toMap(Deployment::zone, Function.identity()),
-                                                                         ImmutableMap::copyOf));
+        return ImmutableMap.copyOf(deployments.values().stream()
+                                           .filter(deployment -> deployment.zone().environment() == Environment.prod)
+                                           .collect(Collectors.toMap(Deployment::zone, Function.identity())));
     }
 
     public DeploymentJobs deploymentJobs() { return deploymentJobs; }
@@ -123,26 +121,20 @@ public class Application {
      */
     public Optional<Version> oldestDeployedVersion() {
         return productionDeployments().values().stream()
-                                               .sorted(Comparator.comparing(Deployment::version))
-                                               .findFirst()
-                                               .map(Deployment::version);
+                .map(Deployment::version)
+                .min(Comparator.naturalOrder());
     }
 
-    /** The version that should be used to compile this application */
-    public Version compileVersion(Controller controller) {
-        return oldestDeployedVersion().orElse(controller.systemVersion());
-    }
-
-    /** Returns the version a deployment to this zone should use for this application */
-    public Version currentDeployVersion(Controller controller, Zone zone) {
+    /** Returns the version a new deployment to this zone should use for this application */
+    public Version deployVersionFor(Zone zone, Controller controller) {
         if (deploying().isPresent() && deploying().get() instanceof VersionChange)
             return ((Change.VersionChange) deploying().get()).version();
 
-        return currentVersion(controller, zone);
+        return currentVersionFor(zone, controller);
     }
 
     /** Returns the current version this application has, or if none; should use, in the given zone */
-    public Version currentVersion(Controller controller, Zone zone) {
+    public Version currentVersionFor(Zone zone, Controller controller) {
         return Optional.ofNullable(deployments().get(zone)).map(Deployment::version) // Already deployed in this zone: Use that version
                 .orElse(oldestDeployedVersion().orElse(controller.systemVersion()));
     }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
index f5bf77cd6d3..0e27ac343ee 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
@@ -288,22 +288,22 @@ public class ApplicationController {
     public ActivateResult deployApplication(ApplicationId applicationId, Zone zone,
                                             ApplicationPackage applicationPackage, DeployOptions options) {
         try (Lock lock = lock(applicationId)) {
-            // Determine what we are doing
             LockedApplication application = get(applicationId, lock).orElse(new LockedApplication(
                     new Application(applicationId), lock)
             );
 
+            // Determine what we are doing
             Version version;
             if (options.deployCurrentVersion)
-                version = application.currentVersion(controller, zone);
+                version = application.currentVersionFor(zone, controller);
             else if (canDeployDirectlyTo(zone, options))
                 version = options.vespaVersion.map(Version::new).orElse(controller.systemVersion());
             else if ( ! application.deploying().isPresent() && ! zone.environment().isManuallyDeployed())
                 return unexpectedDeployment(applicationId, zone, applicationPackage);
             else
-                version = application.currentDeployVersion(controller, zone);
+                version = application.deployVersionFor(zone, controller);
 
-            Optional<DeploymentJobs.JobType> jobType = DeploymentJobs.JobType.from(controller.zoneRegistry().system(), zone);
+            Optional<DeploymentJobs.JobType> jobType = DeploymentJobs.JobType.from(controller.system(), zone);
             ApplicationRevision revision = toApplicationPackageRevision(applicationPackage, options.screwdriverBuildJob);
 
             if ( ! options.deployCurrentVersion) {
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java
index 7cec193992f..52762700cfd 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java
@@ -68,8 +68,8 @@ public class LockedApplication extends Application {
         return new LockedApplication(new Application(id(), deploymentSpec(), validationOverrides(), deployments(),
                                                      deploymentJobs().withTriggering(type,
                                                                                      change,
-                                                                                     determineTriggerVersion(type, controller),
-                                                                                     determineTriggerRevision(type, controller),
+                                                                                     deployVersionFor(type, controller),
+                                                                                     deployRevisionFor(type, controller),
                                                                                      reason,
                                                                                      triggerTime),
                                                      deploying(), hasOutstandingChange()), lock);
@@ -128,28 +128,28 @@ public class LockedApplication extends Application {
                                                      deploymentJobs(), deploying(), outstandingChange), lock);
     }
 
-    private Version determineTriggerVersion(DeploymentJobs.JobType jobType, Controller controller) {
+    private Version deployVersionFor(DeploymentJobs.JobType jobType, Controller controller) {
         return jobType == JobType.component
                ? controller.systemVersion()
-               : currentDeployVersion(controller, jobType.zone(controller.system()).get());
+               : deployVersionFor(jobType.zone(controller.system()).get(), controller);
     }
 
-    private Optional<ApplicationRevision> determineTriggerRevision(DeploymentJobs.JobType jobType, Controller controller) {
+    private Optional<ApplicationRevision> deployRevisionFor(DeploymentJobs.JobType jobType, Controller controller) {
         return jobType == JobType.component
                ? Optional.empty()
-               : currentDeployRevision(jobType.zone(controller.system()).get());
+               : deployRevisionFor(jobType.zone(controller.system()).get());
     }
 
-    /** Returns the version a deployment to this zone should use for this application, or empty if we don't know */
-    private Optional<ApplicationRevision> currentDeployRevision(Zone zone) {
+    /** Returns the revision a new deployment to this zone should use for this application, or empty if we don't know */
+    private Optional<ApplicationRevision> deployRevisionFor(Zone zone) {
         if (deploying().isPresent() && deploying().get() instanceof ApplicationChange)
             return ((Change.ApplicationChange) deploying().get()).revision();
 
-        return currentRevision(zone);
+        return revisionIn(zone);
     }
 
     /** Returns the revision this application is or should be deployed with in the given zone, or empty if unknown. */
-    private Optional<ApplicationRevision> currentRevision(Zone zone) {
+    private Optional<ApplicationRevision> revisionIn(Zone zone) {
         return Optional.ofNullable(deployments().get(zone)).map(Deployment::revision);
     }
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java
index 2089f03a4b3..8704722fa72 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java
@@ -197,6 +197,7 @@ public class DeploymentTrigger {
             Version targetVersion = ((Change.VersionChange)change).version();
             if ( ! (targetVersion.equals(previous.lastSuccess().get().version())) )
                 return false; // version is outdated
+            // The below is checked again in allowedTriggering, right before actual triggering.
             if (next != null && isOnNewerVersionInProductionThan(targetVersion, application, next.type()))
                 return false; // Don't downgrade
         }
@@ -329,10 +330,9 @@ public class DeploymentTrigger {
         //       this leads to some additional corner cases, and the possibility of blocking an application
         //       fix to a version upgrade, so not doing it now
         if (application.deploying().isPresent()) {
-            Change change = application.deploying().get();
-            if (jobType.isProduction() && change.blockedBy(application.deploymentSpec(), clock.instant())) return false;
-            if (change instanceof VersionChange &&
-                    isOnNewerVersionInProductionThan(((VersionChange) change).version(), application, jobType)) return false;
+            if (jobType.isProduction() && application.deploying().get().blockedBy(application.deploymentSpec(), clock.instant())) return false;
+            if (application.deploying().get() instanceof VersionChange &&
+                    isOnNewerVersionInProductionThan(((VersionChange) application.deploying().get()).version(), application, jobType)) return false;
         }
         if (application.deploymentJobs().isRunning(jobType, jobTimeoutLimit())) return false;
         if  ( ! deploysTo(application, jobType)) return false;
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 1bb116809f7..56d7406b080 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
@@ -373,7 +373,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         }
 
         // Compile version. The version that should be used when building an application
-        object.setString("compileVersion", application.compileVersion(controller).toFullString());
+        object.setString("compileVersion", application.oldestDeployedVersion().orElse(controller.systemVersion()).toFullString());
 
         // Rotations
         Cursor globalRotationsArray = object.setArray("globalRotations");