Add obsoletion field to ApplicationVersion, and remove test-only factories

5 files changed, 33 insertions, 26 deletions

diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/ApplicationVersion.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/ApplicationVersion.java
index 04604ae7007..eb2005bf268 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/ApplicationVersion.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/ApplicationVersion.java
@@ -1,11 +1,9 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.api.integration.deployment;
 
-import ai.vespa.validation.Validation;
 import com.yahoo.component.Version;
 
 import java.time.Instant;
-import java.util.Objects;
 import java.util.Optional;
 import java.util.OptionalLong;
 
@@ -33,6 +31,7 @@ public class ApplicationVersion implements Comparable<ApplicationVersion> {
     private final Optional<String> sourceUrl;
     private final Optional<String> commit;
     private final Optional<String> bundleHash;
+    private final Optional<Instant> obsoleteAt;
     private final boolean hasPackage;
     private final boolean shouldSkip;
     private final Optional<String> description;
@@ -41,7 +40,7 @@ public class ApplicationVersion implements Comparable<ApplicationVersion> {
     public ApplicationVersion(RevisionId id, Optional<SourceRevision> source, Optional<String> authorEmail,
                               Optional<Version> compileVersion, Optional<Integer> allowedMajor, Optional<Instant> buildTime,
                               Optional<String> sourceUrl, Optional<String> commit, Optional<String> bundleHash,
-                              boolean hasPackage, boolean shouldSkip, Optional<String> description, int risk) {
+                              Optional<Instant> obsoleteAt, boolean hasPackage, boolean shouldSkip, Optional<String> description, int risk) {
 
         if (commit.isPresent() && commit.get().length() > 128)
             throw new IllegalArgumentException("Commit may not be longer than 128 characters");
@@ -61,6 +60,7 @@ public class ApplicationVersion implements Comparable<ApplicationVersion> {
         this.sourceUrl = requireNonNull(sourceUrl, "sourceUrl cannot be null");
         this.commit = requireNonNull(commit, "commit cannot be null");
         this.bundleHash = bundleHash;
+        this.obsoleteAt = obsoleteAt;
         this.hasPackage = hasPackage;
         this.shouldSkip = shouldSkip;
         this.description = description;
@@ -71,19 +71,9 @@ public class ApplicationVersion implements Comparable<ApplicationVersion> {
         return id;
     }
 
-    /** Create an application package version from a completed build, without an author email */
-    public static ApplicationVersion from(RevisionId id, SourceRevision source) {
-        return new ApplicationVersion(id, Optional.of(source), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), true, false, Optional.empty(), 0);
-    }
-
-    /** Creates a version from a completed build, an author email, and build metadata. */
-    public static ApplicationVersion from(RevisionId id, SourceRevision source, String authorEmail, Version compileVersion, Instant buildTime) {
-        return new ApplicationVersion(id, Optional.of(source), Optional.of(authorEmail), Optional.of(compileVersion), Optional.empty(), Optional.of(buildTime), Optional.empty(), Optional.empty(), Optional.empty(), true, false, Optional.empty(), 0);
-    }
-
     /** Creates a minimal version for a development build. */
     public static ApplicationVersion forDevelopment(RevisionId id, Optional<Version> compileVersion, Optional<Integer> allowedMajor) {
-        return new ApplicationVersion(id, Optional.empty(), Optional.empty(), compileVersion, allowedMajor, Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), true, false, Optional.empty(), 0);
+        return new ApplicationVersion(id, Optional.empty(), Optional.empty(), compileVersion, allowedMajor, Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), true, false, Optional.empty(), 0);
     }
 
     /** Creates a version from a completed build, an author email, and build metadata. */
@@ -91,7 +81,7 @@ public class ApplicationVersion implements Comparable<ApplicationVersion> {
                                                    Optional<Version> compileVersion, Optional<Integer> allowedMajor, Optional<Instant> buildTime, Optional<String> sourceUrl,
                                                    Optional<String> commit, Optional<String> bundleHash, Optional<String> description, int risk) {
         return new ApplicationVersion(id, source, authorEmail, compileVersion, allowedMajor, buildTime,
-                                      sourceUrl, commit, bundleHash, true, false, description, risk);
+                                      sourceUrl, commit, bundleHash, Optional.empty(), true, false, description, risk);
     }
 
     /** Returns a unique identifier for this version or "unknown" if version is not known */
@@ -150,7 +140,17 @@ public class ApplicationVersion implements Comparable<ApplicationVersion> {
 
     /** Returns a copy of this without a package stored. */
     public ApplicationVersion withoutPackage() {
-        return new ApplicationVersion(id, source, authorEmail, compileVersion, allowedMajor, buildTime, sourceUrl, commit, bundleHash, false, shouldSkip, description, risk);
+        return new ApplicationVersion(id, source, authorEmail, compileVersion, allowedMajor, buildTime, sourceUrl, commit, bundleHash, obsoleteAt, false, shouldSkip, description, risk);
+    }
+
+    /** Returns a copy of this which is obsolete now. */
+    public ApplicationVersion obsoleteAt(Instant now) {
+        return new ApplicationVersion(id, source, authorEmail, compileVersion, allowedMajor, buildTime, sourceUrl, commit, bundleHash, Optional.of(now), hasPackage, shouldSkip, description, risk);
+    }
+
+    /** Returns the instant at which this became obsolete, i.e., no longer relevant for automated deployments. */
+    public Optional<Instant> obsoleteAt() {
+        return obsoleteAt;
     }
 
     /** Whether we still have the package for this revision. */
@@ -160,7 +160,7 @@ public class ApplicationVersion implements Comparable<ApplicationVersion> {
 
     /** Returns a copy of this which will not be rolled out to production. */
     public ApplicationVersion skipped() {
-        return new ApplicationVersion(id, source, authorEmail, compileVersion, allowedMajor, buildTime, sourceUrl, commit, bundleHash, hasPackage, true, description, risk);
+        return new ApplicationVersion(id, source, authorEmail, compileVersion, allowedMajor, buildTime, sourceUrl, commit, bundleHash, obsoleteAt, hasPackage, true, description, risk);
     }
 
     /** Whether we still have the package for this revision. */
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RevisionHistory.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RevisionHistory.java
index bbab9487ea2..272417ba0ac 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RevisionHistory.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RevisionHistory.java
@@ -93,7 +93,7 @@ public class RevisionHistory {
 
     // Fallback for when an application version isn't known for the given key.
     private static ApplicationVersion revisionOf(RevisionId id) {
-        return new ApplicationVersion(id, Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), false, false, Optional.empty(), 0);
+        return new ApplicationVersion(id, Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), false, false, Optional.empty(), 0);
     }
 
     /** Returns the production {@link ApplicationVersion} with this revision ID. */
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 ee12c9957b1..5ae7a60d777 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
@@ -118,6 +118,7 @@ public class ApplicationSerializer {
     private static final String riskField = "risk";
     private static final String authorEmailField = "authorEmailField";
     private static final String deployedDirectlyField = "deployedDirectly";
+    private static final String obsoleteAtField = "obsoleteAt";
     private static final String hasPackageField = "hasPackage";
     private static final String shouldSkipField = "shouldSkip";
     private static final String compileVersionField = "compileVersion";
@@ -265,6 +266,7 @@ public class ApplicationSerializer {
         applicationVersion.sourceUrl().ifPresent(url -> object.setString(sourceUrlField, url));
         applicationVersion.commit().ifPresent(commit -> object.setString(commitField, commit));
         object.setBool(deployedDirectlyField, applicationVersion.isDeployedDirectly());
+        applicationVersion.obsoleteAt().ifPresent(at -> object.setLong(obsoleteAtField, at.toEpochMilli()));
         object.setBool(hasPackageField, applicationVersion.hasPackage());
         object.setBool(shouldSkipField, applicationVersion.shouldSkip());
         applicationVersion.description().ifPresent(description -> object.setString(descriptionField, description));
@@ -487,6 +489,7 @@ public class ApplicationSerializer {
         Optional<Instant> buildTime = SlimeUtils.optionalInstant(object.field(buildTimeField));
         Optional<String> sourceUrl = SlimeUtils.optionalString(object.field(sourceUrlField));
         Optional<String> commit = SlimeUtils.optionalString(object.field(commitField));
+        Optional<Instant> obsoleteAt = SlimeUtils.optionalInstant(object.field(obsoleteAtField));
         boolean hasPackage = object.field(hasPackageField).asBool();
         boolean shouldSkip = object.field(shouldSkipField).asBool();
         Optional<String> description = SlimeUtils.optionalString(object.field(descriptionField));
@@ -494,7 +497,7 @@ public class ApplicationSerializer {
         Optional<String> bundleHash = SlimeUtils.optionalString(object.field(bundleHashField));
 
         return new ApplicationVersion(id, sourceRevision, authorEmail, compileVersion, allowedMajor, buildTime,
-                                      sourceUrl, commit, bundleHash, hasPackage, shouldSkip, description, risk);
+                                      sourceUrl, commit, bundleHash, obsoleteAt, hasPackage, shouldSkip, description, risk);
     }
 
     private Optional<SourceRevision> sourceRevisionFromSlime(Inspector object) {
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 a9a6fe602b6..96e35ce876d 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
@@ -106,9 +106,11 @@ public class ControllerTest {
         Version version1 = tester.configServer().initialVersion();
         var context = tester.newDeploymentContext();
         context.submit(applicationPackage);
-        assertEquals(ApplicationVersion.from(RevisionId.forProduction(1), DeploymentContext.defaultSourceRevision, "a@b", new Version("6.1"), Instant.ofEpochSecond(1)),
-                context.application().revisions().get(context.instance().change().revision().get()),
-                "Application version is known from completion of initial job");
+        RevisionId id = RevisionId.forProduction(1);
+        Version compileVersion = new Version("6.1");
+        assertEquals(new ApplicationVersion(id, Optional.of(DeploymentContext.defaultSourceRevision), Optional.of("a@b"), Optional.of(compileVersion), Optional.empty(), Optional.of(Instant.ofEpochSecond(1)), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), true, false, Optional.empty(), 0),
+                     context.application().revisions().get(context.instance().change().revision().get()),
+                     "Application version is known from completion of initial job");
         context.runJob(systemTest);
         context.runJob(stagingTest);
 
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 589fc25700f..5c3f51f5a4f 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
@@ -101,16 +101,17 @@ public class ApplicationSerializerTest {
                 Optional.empty(),
                 Optional.of("best commit"),
                 Optional.of("hash1"),
+                Optional.of(Instant.ofEpochMilli(777)),
                 true,
                 false,
                 Optional.of("~(˘▾˘)~"),
                 3);
         assertEquals("https://github/org/repo/tree/commit1", applicationVersion1.sourceUrl().get());
 
-        ApplicationVersion applicationVersion2 = ApplicationVersion.from(RevisionId.forDevelopment(31, new JobId(id1, DeploymentContext.productionUsEast3)),
-                new SourceRevision("repo1", "branch1", "commit1"), "a@b",
-                Version.fromString("6.3.1"),
-                Instant.ofEpochMilli(496));
+        RevisionId id = RevisionId.forDevelopment(31, new JobId(id1, DeploymentContext.productionUsEast3));
+        SourceRevision source = new SourceRevision("repo1", "branch1", "commit1");
+        Version compileVersion = Version.fromString("6.3.1");
+        ApplicationVersion applicationVersion2 = new ApplicationVersion(id, Optional.of(source), Optional.of("a@b"), Optional.of(compileVersion), Optional.empty(), Optional.of(Instant.ofEpochMilli(496)), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), true, false, Optional.empty(), 0);
         Instant activityAt = Instant.parse("2018-06-01T10:15:30.00Z");
         deployments.add(new Deployment(zone1, CloudAccount.empty, applicationVersion1.id(), Version.fromString("1.2.3"), Instant.ofEpochMilli(3),
                 DeploymentMetrics.none, DeploymentActivity.none, QuotaUsage.none, OptionalDouble.empty()));
@@ -174,6 +175,7 @@ public class ApplicationSerializerTest {
         assertEquals(original.revisions().last().get().sourceUrl(), serialized.revisions().last().get().sourceUrl());
         assertEquals(original.revisions().last().get().commit(), serialized.revisions().last().get().commit());
         assertEquals(original.revisions().last().get().bundleHash(), serialized.revisions().last().get().bundleHash());
+        assertEquals(original.revisions().last().get().obsoleteAt(), serialized.revisions().last().get().obsoleteAt());
         assertEquals(original.revisions().last().get().hasPackage(), serialized.revisions().last().get().hasPackage());
         assertEquals(original.revisions().last().get().shouldSkip(), serialized.revisions().last().get().shouldSkip());
         assertEquals(original.revisions().last().get().description(), serialized.revisions().last().get().description());