Add all versions that are deployed

4 files changed, 35 insertions, 18 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 d97b1b6547d..bb7863a7b69 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
@@ -29,7 +29,9 @@ import java.util.Optional;
 import java.util.OptionalInt;
 import java.util.OptionalLong;
 import java.util.Set;
+import java.util.SortedSet;
 import java.util.TreeMap;
+import java.util.TreeSet;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
@@ -47,7 +49,7 @@ public class Application {
     private final DeploymentSpec deploymentSpec;
     private final ValidationOverrides validationOverrides;
     private final Optional<ApplicationVersion> latestVersion;
-    private final List<ApplicationVersion> versions;
+    private final SortedSet<ApplicationVersion> versions;
     private final OptionalLong projectId;
     private final Optional<IssueId> deploymentIssueId;
     private final Optional<IssueId> ownershipIssueId;
@@ -61,14 +63,14 @@ public class Application {
     public Application(TenantAndApplicationId id, Instant now) {
         this(id, now, DeploymentSpec.empty, ValidationOverrides.empty,
              Optional.empty(), Optional.empty(), Optional.empty(), OptionalInt.empty(),
-             new ApplicationMetrics(0, 0), Set.of(), OptionalLong.empty(), Optional.empty(), List.of(), List.of());
+             new ApplicationMetrics(0, 0), Set.of(), OptionalLong.empty(), Optional.empty(), new TreeSet<>(), List.of());
     }
 
     // DO NOT USE! For serialization purposes, only.
     public Application(TenantAndApplicationId id, Instant createdAt, DeploymentSpec deploymentSpec, ValidationOverrides validationOverrides,
                        Optional<IssueId> deploymentIssueId, Optional<IssueId> ownershipIssueId, Optional<User> owner,
                        OptionalInt majorVersion, ApplicationMetrics metrics, Set<PublicKey> deployKeys, OptionalLong projectId,
-                       Optional<ApplicationVersion> latestVersion, List<ApplicationVersion> versions, Collection<Instance> instances) {
+                       Optional<ApplicationVersion> latestVersion, SortedSet<ApplicationVersion> versions, Collection<Instance> instances) {
         this.id = Objects.requireNonNull(id, "id cannot be null");
         this.createdAt = Objects.requireNonNull(createdAt, "instant of creation cannot be null");
         this.deploymentSpec = Objects.requireNonNull(deploymentSpec, "deploymentSpec cannot be null");
@@ -111,7 +113,7 @@ public class Application {
     public Optional<ApplicationVersion> latestVersion() { return latestVersion; }
 
     /** Returns the currently deployed versions of the application */
-    public List<ApplicationVersion> versions() {
+    public SortedSet<ApplicationVersion> versions() {
         return versions;
     }
 
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 f476ae649f6..06ff381e4dc 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
@@ -13,16 +13,16 @@ import com.yahoo.vespa.hosted.controller.metric.ApplicationMetrics;
 
 import java.security.PublicKey;
 import java.time.Instant;
-import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.LinkedHashSet;
-import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.OptionalInt;
 import java.util.OptionalLong;
 import java.util.Set;
+import java.util.SortedSet;
+import java.util.TreeSet;
 import java.util.function.UnaryOperator;
 
 /**
@@ -45,7 +45,7 @@ public class LockedApplication {
     private final Set<PublicKey> deployKeys;
     private final OptionalLong projectId;
     private final Optional<ApplicationVersion> latestVersion;
-    private final List<ApplicationVersion> versions;
+    private final SortedSet<ApplicationVersion> versions;
     private final Map<InstanceName, Instance> instances;
 
     /**
@@ -66,7 +66,7 @@ public class LockedApplication {
                               ValidationOverrides validationOverrides,
                               Optional<IssueId> deploymentIssueId, Optional<IssueId> ownershipIssueId, Optional<User> owner,
                               OptionalInt majorVersion, ApplicationMetrics metrics, Set<PublicKey> deployKeys,
-                              OptionalLong projectId, Optional<ApplicationVersion> latestVersion, List<ApplicationVersion> versions,
+                              OptionalLong projectId, Optional<ApplicationVersion> latestVersion, SortedSet<ApplicationVersion> versions,
                               Map<InstanceName, Instance> instances) {
         this.lock = lock;
         this.id = id;
@@ -117,8 +117,8 @@ public class LockedApplication {
     }
 
     public LockedApplication withNewSubmission(ApplicationVersion latestVersion) {
-        List<ApplicationVersion> applicationVersions = new ArrayList<>(versions);
-        if (!applicationVersions.contains(latestVersion)) applicationVersions.add(latestVersion);
+        SortedSet<ApplicationVersion> applicationVersions = new TreeSet<>(versions);
+        applicationVersions.add(latestVersion);
         return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides,
                                      deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, deployKeys,
                                      projectId, Optional.of(latestVersion), applicationVersions, instances);
@@ -191,7 +191,7 @@ public class LockedApplication {
     }
 
     public LockedApplication withoutVersion(ApplicationVersion version) {
-        List<ApplicationVersion> applicationVersions = new ArrayList<>(versions);
+        SortedSet<ApplicationVersion> applicationVersions = new TreeSet<>(versions);
         applicationVersions.remove(version);
         return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides,
                 deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, deployKeys,
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java
index 8fc28588662..30b6920fd79 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java
@@ -49,6 +49,8 @@ import java.util.Optional;
 import java.util.OptionalInt;
 import java.util.OptionalLong;
 import java.util.Set;
+import java.util.SortedSet;
+import java.util.TreeSet;
 
 /**
  * Serializes {@link Application}s to/from slime.
@@ -166,7 +168,7 @@ public class ApplicationSerializer {
         root.setDouble(writeQualityField, application.metrics().writeServiceQuality());
         deployKeysToSlime(application.deployKeys(), root.setArray(pemDeployKeysField));
         application.latestVersion().ifPresent(version -> toSlime(version, root.setObject(latestVersionField)));
-        versionsToSlime(application.versions(), root.setArray(versionsField));
+        versionsToSlime(application, root.setArray(versionsField));
         instancesToSlime(application, root.setArray(instancesField));
         return slime;
     }
@@ -226,8 +228,16 @@ public class ApplicationSerializer {
         object.setString(regionField, zone.region().value());
     }
 
-    private void versionsToSlime(List<ApplicationVersion> versions, Cursor object) {
-        versions.forEach(version -> toSlime(version, object.addObject()));
+    private void versionsToSlime(Application application, Cursor object) {
+        // Add all deployed versions as well
+        application.instances()
+                .values()
+                .stream()
+                .flatMap(instance -> instance.deployments().values().stream())
+                .map(Deployment::applicationVersion)
+                .forEach(application.versions()::add);
+
+        application.versions().forEach(version -> toSlime(version, object.addObject()));
     }
 
     private void toSlime(ApplicationVersion applicationVersion, Cursor object) {
@@ -319,7 +329,7 @@ public class ApplicationSerializer {
         List<Instance> instances = instancesFromSlime(id, root.field(instancesField));
         OptionalLong projectId = SlimeUtils.optionalLong(root.field(projectIdField));
         Optional<ApplicationVersion> latestVersion = latestVersionFromSlime(root.field(latestVersionField));
-        List<ApplicationVersion> versions = versionsFromSlime(root.field(versionsField));
+        SortedSet<ApplicationVersion> versions = versionsFromSlime(root.field(versionsField));
 
         return new Application(id, createdAt, deploymentSpec, validationOverrides,
                                deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics,
@@ -331,8 +341,8 @@ public class ApplicationSerializer {
                        .filter(version -> ! version.isUnknown());
     }
 
-    private List<ApplicationVersion> versionsFromSlime(Inspector versionsObject) {
-        List<ApplicationVersion> versions = new ArrayList<>();
+    private SortedSet<ApplicationVersion> versionsFromSlime(Inspector versionsObject) {
+        SortedSet<ApplicationVersion> versions = new TreeSet<>();
         versionsObject.traverse((ArrayTraverser) (name, object) -> versions.add(applicationVersionFromSlime(object)));
         return versions;
     }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java
index dda0e219e7b..06c9daa1892 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java
@@ -42,6 +42,8 @@ import java.util.OptionalDouble;
 import java.util.OptionalInt;
 import java.util.OptionalLong;
 import java.util.Set;
+import java.util.SortedSet;
+import java.util.TreeSet;
 
 import static org.junit.Assert.assertEquals;
 
@@ -91,7 +93,7 @@ public class ApplicationSerializerTest {
                                                                         Optional.of("best commit"),
                                                                         true,
                                                                         Optional.of("hash1"));
-        List<ApplicationVersion> versions = List.of(applicationVersion1);
+        SortedSet<ApplicationVersion> versions = new TreeSet<>(Set.of(applicationVersion1));
         assertEquals("https://github/org/repo/tree/commit1", applicationVersion1.sourceUrl().get());
 
         ApplicationVersion applicationVersion2 = ApplicationVersion
@@ -156,7 +158,10 @@ public class ApplicationSerializerTest {
         assertEquals(original.latestVersion().get().buildTime(), serialized.latestVersion().get().buildTime());
         assertEquals(original.latestVersion().get().sourceUrl(), serialized.latestVersion().get().sourceUrl());
         assertEquals(original.latestVersion().get().commit(), serialized.latestVersion().get().commit());
+        // Check deployed versions are added
         assertEquals(original.versions(), serialized.versions());
+        assertEquals(serialized.versions(), new TreeSet<>(Set.of(applicationVersion1, applicationVersion2)));
+
 
         assertEquals(original.deploymentSpec().xmlForm(), serialized.deploymentSpec().xmlForm());
         assertEquals(original.validationOverrides().xmlForm(), serialized.validationOverrides().xmlForm());