1 files changed, 21 insertions, 40 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationVersion.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationVersion.java
index 31ee5fa6d44..304d82b2bec 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationVersion.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationVersion.java
@@ -11,19 +11,18 @@ import java.util.Optional;
  * @author bratseth
  * @author mpolden
  */
-public class ApplicationVersion implements Comparable<ApplicationVersion> {
+public class ApplicationVersion {
 
-    /**
-     * Used in cases where application version cannot be determined, such as manual deployments (e.g. in dev
-     * environment)
-     */
+    // TODO: Remove the need for this
     public static final ApplicationVersion unknown = new ApplicationVersion();
 
-    // This never changes and is only used to create a valid semantic version number, as required by application bundles
+    // Never changes. Only used to create a valid version number for the bundle
     private static final String majorVersion = "1.0";
 
-    // TODO: Remove after 2018-03-01
+    // TODO: Remove after introducing new application version
     private final Optional<String> applicationPackageHash;
+
+    // TODO: Make mandatory
     private final Optional<SourceRevision> source;
     private final Optional<Long> buildNumber;
 
@@ -38,11 +37,11 @@ public class ApplicationVersion implements Comparable<ApplicationVersion> {
         Objects.requireNonNull(applicationPackageHash, "applicationPackageHash cannot be null");
         Objects.requireNonNull(source, "source cannot be null");
         Objects.requireNonNull(buildNumber, "buildNumber cannot be null");
-        if (!applicationPackageHash.isPresent() && source.isPresent() != buildNumber.isPresent()) {
-            throw new IllegalArgumentException("both buildNumber and source must be set together");
+        if (buildNumber.isPresent() && !source.isPresent()) {
+            throw new IllegalArgumentException("both buildNumber and source must be set if buildNumber is set");
         }
-        if (buildNumber.isPresent() && buildNumber.get() <= 0) {
-            throw new IllegalArgumentException("buildNumber must be > 0");
+        if ( ! buildNumber.isPresent() && ! applicationPackageHash.isPresent()) {
+            throw new IllegalArgumentException("applicationPackageHash must be given if buildNumber is unset");
         }
         this.applicationPackageHash = applicationPackageHash;
         this.source = source;
@@ -69,14 +68,9 @@ public class ApplicationVersion implements Comparable<ApplicationVersion> {
         if (applicationPackageHash.isPresent()) {
             return applicationPackageHash.get();
         }
-        return source.map(sourceRevision -> String.format("%s.%d-%s", majorVersion, buildNumber.get(),
-                                                          abbreviateCommit(source.get().commit())))
-                     .orElse("unknown");
+        return String.format("%s.%d-%s", majorVersion, buildNumber.get(), abbreviateCommit(source.get().commit()));
     }
 
-    /** Returns the application package hash, if known */
-    public Optional<String> applicationPackageHash() { return applicationPackageHash; }
-
     /**
      * Returns information about the source of this revision, or empty if the source is not know/defined
      * (which is the case for command-line deployment from developers, but never for deployment jobs)
@@ -86,29 +80,23 @@ public class ApplicationVersion implements Comparable<ApplicationVersion> {
     /** Returns the build number that built this version */
     public Optional<Long> buildNumber() { return buildNumber; }
 
-    /** Returns whether this is unknown */
-    public boolean isUnknown() {
-        return this.equals(unknown);
-    }
-
     @Override
-    public boolean equals(Object o) {
-        if (this == o) return true;
-        if (o == null || getClass() != o.getClass()) return false;
-        ApplicationVersion that = (ApplicationVersion) o;
-        return Objects.equals(applicationPackageHash, that.applicationPackageHash) &&
-               Objects.equals(source, that.source) &&
-               Objects.equals(buildNumber, that.buildNumber);
-    }
+    public int hashCode() { return applicationPackageHash.hashCode(); }
 
     @Override
-    public int hashCode() {
-        return Objects.hash(applicationPackageHash, source, buildNumber);
+    public boolean equals(Object other) {
+        if (this == other) return true;
+        if ( ! (other instanceof ApplicationVersion)) return false;
+        return this.applicationPackageHash.equals(((ApplicationVersion)other).applicationPackageHash);
     }
 
     @Override
     public String toString() {
-        return "Application package version: " + id() + source.map(s -> ", " + s.toString()).orElse("");
+        if (buildNumber.isPresent()) {
+            return "Application package version: " + abbreviateCommit(source.get().commit()) + "-" + buildNumber.get();
+        }
+        return "Application package revision '" + applicationPackageHash + "'" +
+               (source.isPresent() ? " with " + source.get() : "");
     }
 
     /** Abbreviate given commit hash to 9 characters */
@@ -116,11 +104,4 @@ public class ApplicationVersion implements Comparable<ApplicationVersion> {
         return hash.length() <= 9 ? hash : hash.substring(0, 9);
     }
 
-    @Override
-    public int compareTo(ApplicationVersion o) {
-        if (!buildNumber().isPresent() || !o.buildNumber().isPresent()) {
-            return 0; // No sorting for application package hash
-        }
-        return buildNumber().get().compareTo(o.buildNumber().get());
-    }
 }