Merge pull request #13296 from vespa-engine/mpolden/inherit-generic-list

Let NodeList inherit AbstractFilteringList

8 files changed, 40 insertions, 86 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/LockedNodeList.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/LockedNodeList.java
index b90906f8974..662bcd33a17 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/LockedNodeList.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/LockedNodeList.java
@@ -24,11 +24,10 @@ public class LockedNodeList extends NodeList {
     private final Mutex lock;
 
     public LockedNodeList(List<Node> nodes, Mutex lock) {
-        super(nodes);
+        super(nodes, false);
         this.lock = Objects.requireNonNull(lock, "lock must be non-null");
     }
 
-    @Override
     public LockedNodeList filter(Predicate<Node> predicate) {
         return asList().stream()
                        .filter(predicate)
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java
index 8121e868369..ce696903a53 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java
@@ -1,6 +1,7 @@
 // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.provision;
 
+import com.yahoo.collections.AbstractFilteringList;
 import com.yahoo.component.Version;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.ClusterSpec;
@@ -8,13 +9,10 @@ import com.yahoo.config.provision.NodeResources;
 import com.yahoo.config.provision.NodeType;
 
 import java.util.Collection;
-import java.util.Collections;
 import java.util.Comparator;
 import java.util.EnumSet;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Optional;
-import java.util.function.Predicate;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -26,114 +24,99 @@ import static java.util.stream.Collectors.collectingAndThen;
  * @author bratseth
  * @author mpolden
  */
-public class NodeList implements Iterable<Node> {
+public class NodeList extends AbstractFilteringList<Node, NodeList> {
 
-    private final List<Node> nodes;
-    private final boolean negate;
-
-    NodeList(List<Node> nodes) {
-        this(nodes, true, false);
-    }
-
-    private NodeList(List<Node> nodes, boolean copy, boolean negate) {
-        this.nodes = copy ? List.copyOf(nodes) : Collections.unmodifiableList(nodes);
-        this.negate = negate;
-    }
-
-    /** Invert the next filter operation. All other methods that return a {@link NodeList} clears the negation. */
-    public NodeList not() {
-        return new NodeList(nodes, false, true);
+    protected NodeList(List<Node> nodes, boolean negate) {
+        super(nodes, negate, NodeList::new);
     }
 
     /** Returns the subset of nodes which are retired */
     public NodeList retired() {
-        return filter(node -> node.allocation().get().membership().retired());
+        return matching(node -> node.allocation().get().membership().retired());
     }
 
     /** Returns the subset of nodes that are being deprovisioned */
     public NodeList deprovisioning() {
-        return filter(node -> node.status().wantToRetire() && node.status().wantToDeprovision());
+        return matching(node -> node.status().wantToRetire() && node.status().wantToDeprovision());
     }
 
     /** Returns the subset of nodes which are removable */
     public NodeList removable() {
-        return filter(node -> node.allocation().get().isRemovable());
+        return matching(node -> node.allocation().get().isRemovable());
     }
 
     /** Returns the subset of nodes having exactly the given resources */
-    public NodeList resources(NodeResources resources) { return filter(node -> node.flavor().resources().equals(resources)); }
+    public NodeList resources(NodeResources resources) { return matching(node -> node.flavor().resources().equals(resources)); }
 
     /** Returns the subset of nodes of the given flavor */
     public NodeList flavor(String flavor) {
-        return filter(node -> node.flavor().name().equals(flavor));
+        return matching(node -> node.flavor().name().equals(flavor));
     }
 
     /** Returns the subset of nodes assigned to the given cluster type */
     public NodeList type(ClusterSpec.Type type) {
-        return filter(node -> node.allocation().isPresent() && node.allocation().get().membership().cluster().type().equals(type));
+        return matching(node -> node.allocation().isPresent() && node.allocation().get().membership().cluster().type().equals(type));
     }
 
     /** Returns the subset of nodes that run containers */
     public NodeList container() {
-        return filter(node -> node.allocation().isPresent() && node.allocation().get().membership().cluster().type().isContainer());
+        return matching(node -> node.allocation().isPresent() && node.allocation().get().membership().cluster().type().isContainer());
     }
 
     /** Returns the subset of nodes that are currently changing their Vespa version */
     public NodeList changingVersion() {
-        return filter(node -> node.status().vespaVersion().isPresent() &&
-                              node.allocation().isPresent() &&
-                              !node.status().vespaVersion().get().equals(node.allocation().get().membership().cluster().vespaVersion()));
+        return matching(node -> node.status().vespaVersion().isPresent() &&
+                                node.allocation().isPresent() &&
+                                !node.status().vespaVersion().get().equals(node.allocation().get().membership().cluster().vespaVersion()));
     }
 
     /** Returns the subset of nodes that are currently changing their OS version to given version */
     public NodeList changingOsVersionTo(Version version) {
-        return filter(node -> node.status().osVersion().changingTo(version));
+        return matching(node -> node.status().osVersion().changingTo(version));
     }
 
     /** Returns the subset of nodes that are currently changing their OS version */
     public NodeList changingOsVersion() {
-        return filter(node -> node.status().osVersion().changing());
+        return matching(node -> node.status().osVersion().changing());
     }
 
     /** Returns a copy of this sorted by current OS version (lowest to highest) */
     public NodeList byIncreasingOsVersion() {
-        return nodes.stream()
-                    .sorted(Comparator.comparing(node -> node.status()
-                                                             .osVersion()
-                                                             .current()
-                                                             .orElse(Version.emptyVersion)))
-                    .collect(collectingAndThen(Collectors.toList(), NodeList::wrap));
+        return sortedBy(Comparator.comparing(node -> node.status()
+                                                         .osVersion()
+                                                         .current()
+                                                         .orElse(Version.emptyVersion)));
     }
 
     /** Returns the subset of nodes that are currently on the given OS version */
     public NodeList onOsVersion(Version version) {
-        return filter(node -> node.status().osVersion().matches(version));
+        return matching(node -> node.status().osVersion().matches(version));
     }
 
     /** Returns the subset of nodes assigned to the given cluster */
     public NodeList cluster(ClusterSpec.Id cluster) {
-        return filter(node -> node.allocation().isPresent() && node.allocation().get().membership().cluster().id().equals(cluster));
+        return matching(node -> node.allocation().isPresent() && node.allocation().get().membership().cluster().id().equals(cluster));
     }
 
     /** Returns the subset of nodes owned by the given application */
     public NodeList owner(ApplicationId application) {
-        return filter(node -> node.allocation().map(a -> a.owner().equals(application)).orElse(false));
+        return matching(node -> node.allocation().map(a -> a.owner().equals(application)).orElse(false));
     }
 
     /** Returns the subset of nodes matching the given node type(s) */
     public NodeList nodeType(NodeType first, NodeType... rest) {
         EnumSet<NodeType> nodeTypes = EnumSet.of(first, rest);
-        return filter(node -> nodeTypes.contains(node.type()));
+        return matching(node -> nodeTypes.contains(node.type()));
     }
 
     /** Returns the subset of nodes that are parents */
     public NodeList parents() {
-        return filter(n -> n.parentHostname().isEmpty());
+        return matching(n -> n.parentHostname().isEmpty());
     }
 
     /** Returns the child nodes of the given parent node */
     public NodeList childrenOf(String hostname) {
-        return filter(n -> n.parentHostname().map(hostname::equals).orElse(false));
+        return matching(n -> n.parentHostname().map(hostname::equals).orElse(false));
     }
 
     public NodeList childrenOf(Node parent) {
@@ -147,7 +130,7 @@ public class NodeList implements Iterable<Node> {
 
     /** Returns the subset of nodes that are in any of the given state(s) */
     public NodeList state(Collection<Node.State> nodeStates) {
-        return filter(node -> nodeStates.contains(node.state()));
+        return matching(node -> nodeStates.contains(node.state()));
     }
 
     /** Returns the parent nodes of the given child nodes */
@@ -156,49 +139,21 @@ public class NodeList implements Iterable<Node> {
                        .map(this::parentOf)
                        .filter(Optional::isPresent)
                        .flatMap(Optional::stream)
-                       .collect(collectingAndThen(Collectors.toList(), NodeList::wrap));
+                       .collect(collectingAndThen(Collectors.toList(), NodeList::copyOf));
     }
 
     /** Returns the parent node of the given child node */
     public Optional<Node> parentOf(Node child) {
         return child.parentHostname()
-                .flatMap(parentHostname -> nodes.stream()
-                        .filter(node -> node.hostname().equals(parentHostname))
-                        .findFirst());
-    }
-
-    /** Returns the first n nodes in this */
-    public NodeList first(int n) {
-        n = Math.min(n, nodes.size());
-        return wrap(nodes.subList(negate ? n : 0,
-                                  negate ? nodes.size() : n));
+                    .flatMap(parentHostname -> stream().filter(node -> node.hostname().equals(parentHostname))
+                                                       .findFirst());
     }
 
-    public int size() { return nodes.size(); }
-
-    /** Returns the immutable list of nodes in this */
-    public List<Node> asList() { return nodes; }
-
     /** Returns the nodes of this as a stream */
     public Stream<Node> stream() { return asList().stream(); }
 
-    public NodeList filter(Predicate<Node> predicate) {
-        return nodes.stream().filter(negate ? predicate.negate() : predicate)
-                    .collect(collectingAndThen(Collectors.toList(), NodeList::wrap));
-    }
-
-    @Override
-    public Iterator<Node> iterator() {
-        return nodes.iterator();
-    }
-
-    /** Create a new list containing the given nodes, without copying */
-    private static NodeList wrap(List<Node> nodes) {
-        return new NodeList(nodes, false, false);
-    }
-
     public static NodeList copyOf(List<Node> nodes) {
-        return new NodeList(nodes, true, false);
+        return new NodeList(nodes, false);
     }
 
 }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsFetcher.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsFetcher.java
index 232fee1df6a..acdd419c0de 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsFetcher.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsFetcher.java
@@ -58,7 +58,7 @@ public class NodeMetricsFetcher extends AbstractComponent implements NodeMetrics
         if (Autoscaler.unstable(applicationNodes.asList())) return Collections.emptyList();
 
         Optional<Node> metricsV2Container = applicationNodes.container()
-                                                            .filter(node -> expectedUp(node))
+                                                            .matching(node -> expectedUp(node))
                                                             .stream()
                                                             .findFirst();
         if (metricsV2Container.isEmpty()) return Collections.emptyList();
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Rebalancer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Rebalancer.java
index 19c612a46c5..f044eaf97f6 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Rebalancer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Rebalancer.java
@@ -90,7 +90,7 @@ public class Rebalancer extends NodeRepositoryMaintainer {
             ApplicationId applicationId = node.allocation().get().owner();
             if (applicationId.instance().isTester()) continue;
             if (deployedRecently(applicationId)) continue;
-            for (Node toHost : allNodes.filter(nodeRepository()::canAllocateTenantNodeTo)) {
+            for (Node toHost : allNodes.matching(nodeRepository()::canAllocateTenantNodeTo)) {
                 if (toHost.hostname().equals(node.parentHostname().get())) continue;
                 if ( ! capacity.freeCapacityOf(toHost).satisfies(node.flavor().resources())) continue;
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java
index aa824e7a930..3210de18b7d 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java
@@ -259,7 +259,7 @@ public class IP {
          */
         public Set<String> findUnused(NodeList nodes) {
             var unusedAddresses = new LinkedHashSet<>(asSet());
-            nodes.filter(node -> node.ipConfig().primary().stream().anyMatch(ip -> asSet().contains(ip)))
+            nodes.matching(node -> node.ipConfig().primary().stream().anyMatch(ip -> asSet().contains(ip)))
                  .forEach(node -> unusedAddresses.removeAll(node.ipConfig().primary()));
             return Collections.unmodifiableSet(unusedAddresses);
         }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java
index 026788e9d6c..c6945e1779b 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java
@@ -6,7 +6,6 @@ import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.config.provision.exception.LoadBalancerServiceException;
-import java.util.logging.Level;
 import com.yahoo.transaction.Mutex;
 import com.yahoo.transaction.NestedTransaction;
 import com.yahoo.vespa.hosted.provision.Node;
@@ -26,6 +25,7 @@ import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
 import java.util.function.Function;
+import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 
@@ -181,9 +181,9 @@ public class LoadBalancerProvisioner {
     private List<Node> allocatedContainers(ApplicationId application, ClusterSpec.Id clusterId) {
         return NodeList.copyOf(nodeRepository.getNodes(NodeType.tenant, Node.State.reserved, Node.State.active))
                        .owner(application)
-                       .filter(node -> node.state().isAllocated())
+                       .matching(node -> node.state().isAllocated())
                        .container()
-                       .filter(node -> effectiveId(node.allocation().get().membership().cluster()).equals(clusterId))
+                       .matching(node -> effectiveId(node.allocation().get().membership().cluster()).equals(clusterId))
                        .asList();
     }
 
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java
index 75d83f00ebb..6b03def7e57 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java
@@ -236,7 +236,7 @@ public class OsVersionsTest {
     }
 
     private NodeList retiringChildrenOf(Node parent) {
-        return tester.nodeRepository().list().childrenOf(parent).filter(child -> child.status().wantToRetire());
+        return tester.nodeRepository().list().childrenOf(parent).matching(child -> child.status().wantToRetire());
     }
 
     private List<Node> provisionInfraApplication(int nodeCount) {
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InPlaceResizeProvisionTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InPlaceResizeProvisionTest.java
index 3f62b263c1d..16fba824300 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InPlaceResizeProvisionTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InPlaceResizeProvisionTest.java
@@ -259,7 +259,7 @@ public class InPlaceResizeProvisionTest {
 
     private NodeList listCluster(ClusterSpec cluster) {
         return tester.getNodes(app, Node.State.active)
-                .filter(node -> node.allocation().get().membership().cluster().satisfies(cluster));
+                     .matching(node -> node.allocation().get().membership().cluster().satisfies(cluster));
     }
 
     private static class PrepareHelper {