Code review fixes

5 files changed, 20 insertions, 31 deletions

diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/artifact/Artifact.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/artifact/Artifact.java
index e0a4e0c0ae8..7ca372f6cd0 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/artifact/Artifact.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/artifact/Artifact.java
@@ -5,7 +5,6 @@ import com.yahoo.component.Version;
 
 import java.time.Instant;
 import java.util.Objects;
-import java.util.Optional;
 
 /**
  * A registry artifact (e.g. container image or RPM)
@@ -15,25 +14,16 @@ import java.util.Optional;
 public class Artifact {
 
     private final String id;
-    private final Optional<String> registry;
-    private final Optional<String> repository;
-    private final Optional<String> tag;
+    private final String registry;
+    private final String repository;
+    private final String tag;
     private final Instant createdAt;
     private final Version version;
 
     public Artifact(String id, String registry, String repository, String tag, Instant createdAt, Version version) {
         this.id = Objects.requireNonNull(id);
-        this.registry = Optional.of(registry);
-        this.repository = Optional.of(repository);
-        this.tag = Optional.of(tag);
-        this.createdAt = Objects.requireNonNull(createdAt);
-        this.version = Objects.requireNonNull(version);
-    }
-
-    public Artifact(String id, Instant createdAt, Optional<String> tag, Version version) {
-        this.id = Objects.requireNonNull(id);
-        this.registry = Optional.empty();
-        this.repository = Optional.empty();
+        this.registry = Objects.requireNonNull(registry);
+        this.repository = Objects.requireNonNull(repository);
         this.tag = Objects.requireNonNull(tag);
         this.createdAt = Objects.requireNonNull(createdAt);
         this.version = Objects.requireNonNull(version);
@@ -45,17 +35,17 @@ public class Artifact {
     }
 
     /** The registry holding this artifact */
-    public Optional<String> registry() {
+    public String registry() {
         return registry;
     }
 
     /** Repository of this artifact */
-    public Optional<String> repository() {
+    public String repository() {
         return repository;
     }
 
     /** Tag of this artifact */
-    public Optional<String> tag() {
+    public String tag() {
         return tag;
     }
 
@@ -89,7 +79,6 @@ public class Artifact {
 
     @Override
     public String toString() {
-        String name = repository.isPresent() ? registry.get() + "/" + repository.get() : id;
-        return "artifact " + name + " [version=" + version.toFullString() + ",createdAt=" + createdAt + tag.map(t -> ",tag=" + t).orElse("") + "]";
+        return "artifact " + registry + "/" + repository + " [version=" + version.toFullString() + ",createdAt=" + createdAt + ",tag=" + tag + "]";
     }
 }
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/artifact/ArtifactRegistry.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/artifact/ArtifactRegistry.java
index 8d3c19083c9..6ab8409ad11 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/artifact/ArtifactRegistry.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/artifact/ArtifactRegistry.java
@@ -12,7 +12,7 @@ import java.util.List;
 public interface ArtifactRegistry {
 
     /** Delete all given artifacts */
-    void deleteAll(List<Artifact> images);
+    void deleteAll(List<Artifact> artifacts);
 
     /** Returns a list of all artifacts in this system */
     List<Artifact> list();
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ArtifactExpirer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ArtifactExpirer.java
index 47eeb291dd7..81fbb85e3d7 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ArtifactExpirer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ArtifactExpirer.java
@@ -37,18 +37,18 @@ public class ArtifactExpirer extends ControllerMaintainer {
 
     @Override
     protected double maintain() {
+        VersionStatus versionStatus = controller().readVersionStatus();
         return controller().clouds().stream()
                 .flatMapToDouble(cloud ->
                     controller().serviceRegistry().artifactRegistry(cloud).stream()
-                            .mapToDouble(artifactRegistry -> maintain(cloud, artifactRegistry)))
+                            .mapToDouble(artifactRegistry -> maintain(versionStatus, cloud, artifactRegistry)))
                 .average()
                 .orElse(1);
     }
 
-    private double maintain(CloudName cloudName, ArtifactRegistry artifactRegistry) {
+    private double maintain(VersionStatus versionStatus, CloudName cloudName, ArtifactRegistry artifactRegistry) {
         try {
             Instant now = controller().clock().instant();
-            VersionStatus versionStatus = controller().readVersionStatus();
             List<Artifact> artifactsToExpire = artifactRegistry.list().stream()
                     .filter(artifact -> isExpired(artifact, now, versionStatus))
                     .collect(Collectors.toList());
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ArtifactRegistryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ArtifactRegistryMock.java
index 1d3974fe7a7..ef0445e06c6 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ArtifactRegistryMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ArtifactRegistryMock.java
@@ -15,9 +15,9 @@ import java.util.stream.Collectors;
  */
 public class ArtifactRegistryMock implements ArtifactRegistry {
 
-    private static final Comparator<Artifact> comparator = Comparator.comparing((Artifact artifact) -> artifact.registry().orElse(""))
-                                                                           .thenComparing(artifact -> artifact.repository().orElse(""))
-                                                                           .thenComparing(Artifact::version);
+    private static final Comparator<Artifact> comparator = Comparator.comparing(Artifact::registry)
+                                                                     .thenComparing(Artifact::repository)
+                                                                     .thenComparing(Artifact::version);
 
     private final Map<String, Artifact> images = new HashMap<>();
 
@@ -27,8 +27,8 @@ public class ArtifactRegistryMock implements ArtifactRegistry {
     }
 
     @Override
-    public void deleteAll(List<Artifact> images) {
-        images.forEach(image -> this.images.remove(image.id()));
+    public void deleteAll(List<Artifact> artifacts) {
+        artifacts.forEach(image -> this.images.remove(image.id()));
     }
 
     @Override
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ArtifactExpirerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ArtifactExpirerTest.java
index 9a19d4d0185..7703266c9ba 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ArtifactExpirerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ArtifactExpirerTest.java
@@ -26,8 +26,8 @@ public class ArtifactExpirerTest {
         ArtifactRegistryMock registry = tester.controllerTester().serviceRegistry().artifactRegistry(CloudName.defaultName()).orElseThrow();
 
         Instant instant = tester.clock().instant();
-        Artifact image0 = new Artifact("image0", "registry.example.com", "vespa/vespa", "7.4", instant, Version.fromString("7.1"));
-        Artifact image1 = new Artifact("image1", "registry.example.com", "vespa/vespa", "7.4-amd64", instant, Version.fromString("7.2"));
+        Artifact image0 = new Artifact("image0", "registry.example.com", "vespa/vespa", "7.1", instant, Version.fromString("7.1"));
+        Artifact image1 = new Artifact("image1", "registry.example.com", "vespa/vespa", "7.2-amd64", instant, Version.fromString("7.2"));
         Artifact image2 = new Artifact("image2", "registry.example.com", "vespa/vespa", "7.4-amd64", instant, Version.fromString("7.4"));
         registry.add(image0)
                 .add(image1)