Cleanup the concept of orderedGroups. Just use a single way of accessing the groups.

Simplify testing by introducing a GroupList to contain all acces to groups and nodes.

6 files changed, 34 insertions, 113 deletions

diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/DispatcherTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/DispatcherTest.java
index bc92afdb8fc..3c5b026b95e 100644
--- a/container-search/src/test/java/com/yahoo/search/dispatch/DispatcherTest.java
+++ b/container-search/src/test/java/com/yahoo/search/dispatch/DispatcherTest.java
@@ -110,7 +110,7 @@ public class DispatcherTest {
     void testGroup0IsSkippedWhenItIsBlockingFeed() {
         SearchCluster cluster = new MockSearchCluster("1", 3, 1);
         Dispatcher dispatcher = new Dispatcher(new ClusterMonitor<>(cluster, false), cluster, dispatchConfig, new MockInvokerFactory(cluster, dispatchConfig, (n, a) -> true));
-        cluster.group(0).get().nodes().get(0).setBlockingWrites(true);
+        cluster.group(0).nodes().get(0).setBlockingWrites(true);
         cluster.pingIterationCompleted();
         assertEquals(1,
                 (dispatcher.getSearchInvoker(new Query(), null).distributionKey().get()).longValue(),
@@ -122,8 +122,8 @@ public class DispatcherTest {
     void testGroup0IsSelectedWhenMoreAreBlockingFeed() {
         SearchCluster cluster = new MockSearchCluster("1", 3, 1);
         Dispatcher dispatcher = new Dispatcher(new ClusterMonitor<>(cluster, false), cluster, dispatchConfig, new MockInvokerFactory(cluster, dispatchConfig, (n, a) -> true));
-        cluster.group(0).get().nodes().get(0).setBlockingWrites(true);
-        cluster.group(1).get().nodes().get(0).setBlockingWrites(true);
+        cluster.group(0).nodes().get(0).setBlockingWrites(true);
+        cluster.group(1).nodes().get(0).setBlockingWrites(true);
         cluster.pingIterationCompleted();
         assertEquals(0,
                 dispatcher.getSearchInvoker(new Query(), null).distributionKey().get().longValue(),
@@ -135,7 +135,7 @@ public class DispatcherTest {
     void testGroup0IsSelectedWhenItIsBlockingFeedWhenNoOthers() {
         SearchCluster cluster = new MockSearchCluster("1", 1, 1);
         Dispatcher dispatcher = new Dispatcher(new ClusterMonitor<>(cluster, false), cluster, dispatchConfig, new MockInvokerFactory(cluster, dispatchConfig, (n, a) -> true));
-        cluster.group(0).get().nodes().get(0).setBlockingWrites(true);
+        cluster.group(0).nodes().get(0).setBlockingWrites(true);
         cluster.pingIterationCompleted();
         assertEquals(0,
                 (dispatcher.getSearchInvoker(new Query(), null).distributionKey().get()).longValue(),
diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/LoadBalancerTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/LoadBalancerTest.java
index 40711fb8dbe..62b56e6e8ff 100644
--- a/container-search/src/test/java/com/yahoo/search/dispatch/LoadBalancerTest.java
+++ b/container-search/src/test/java/com/yahoo/search/dispatch/LoadBalancerTest.java
@@ -11,9 +11,10 @@ import org.opentest4j.AssertionFailedError;
 
 import java.time.Duration;
 import java.time.Instant;
-import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.Optional;
 import java.util.Random;
 
@@ -110,14 +111,19 @@ public class LoadBalancerTest {
         assertEquals(Duration.ofNanos(1045087), decayer.averageSearchTime());
     }
 
+    private Map<Integer, GroupStatus> createScoreBoard(int count) {
+        Map<Integer, GroupStatus> scoreboard = new HashMap<>();
+        for (int i = 0; i < count; i++) {
+            GroupStatus gs = newGroupStatus(i);
+            scoreboard.put(gs.groupId(), gs);
+        }
+        return scoreboard;
+    }
+
     @Test
     void requireEqualDistributionInFlatWeightListWithAdaptiveScheduler() {
-        List<GroupStatus> scoreboard = new ArrayList<>();
-        for (int i = 0; i < 5; i++) {
-            scoreboard.add(newGroupStatus(i));
-        }
         Random seq = sequence(0.0, 0.1, 0.2, 0.39, 0.4, 0.6, 0.8, 0.99999);
-        AdaptiveScheduler sched = new AdaptiveScheduler(AdaptiveScheduler.Type.REQUESTS, seq, scoreboard);
+        AdaptiveScheduler sched = new AdaptiveScheduler(AdaptiveScheduler.Type.REQUESTS, seq, createScoreBoard(5));
 
         assertEquals(0, sched.takeNextGroup(null).get().groupId());
         assertEquals(0, sched.takeNextGroup(null).get().groupId());
@@ -131,15 +137,11 @@ public class LoadBalancerTest {
 
     @Test
     void requireThatAdaptiveSchedulerObeysWeights() {
-        List<GroupStatus> scoreboard = new ArrayList<>();
-        for (int i = 0; i < 5; i++) {
-            GroupStatus gs = newGroupStatus(i);
-            scoreboard.add(gs);
-        }
+        var scoreboard = createScoreBoard(5);
         Random seq = sequence(0.0, 0.4379, 0.4380, 0.6569, 0.6570, 0.8029, 0.8030, 0.9124, 0.9125);
         AdaptiveScheduler sched = new AdaptiveScheduler(AdaptiveScheduler.Type.REQUESTS, seq, scoreboard);
-        int i= 0;
-        for (GroupStatus gs : scoreboard) {
+        int i = 0;
+        for (GroupStatus gs : scoreboard.values()) {
             gs.setDecayer(new AdaptiveScheduler.DecayByRequests(1, Duration.ofMillis((long)(0.1 * (i + 1)*1000.0))));
             i++;
         }
@@ -161,10 +163,6 @@ public class LoadBalancerTest {
     }
     @Test
     void requireBestOfRandom2Scheduler() {
-        List<GroupStatus> scoreboard = new ArrayList<>();
-        for (int i = 0; i < 5; i++) {
-            scoreboard.add(newGroupStatus(i));
-        }
         Random seq = sequence(
                 0.1, 0.125,
                 0.1, 0.125,
@@ -175,7 +173,7 @@ public class LoadBalancerTest {
                 0.9, 0.125,
                 0.9, 0.125
                 );
-        BestOfRandom2 sched = new BestOfRandom2(seq, scoreboard);
+        BestOfRandom2 sched = new BestOfRandom2(seq, createScoreBoard(5));
 
         assertEquals(0, allocate(sched.takeNextGroup(null).get()).groupId());
         assertEquals(1, allocate(sched.takeNextGroup(null).get()).groupId());
diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/MockSearchCluster.java b/container-search/src/test/java/com/yahoo/search/dispatch/MockSearchCluster.java
index 32ca63693b4..9a4931a8fa7 100644
--- a/container-search/src/test/java/com/yahoo/search/dispatch/MockSearchCluster.java
+++ b/container-search/src/test/java/com/yahoo/search/dispatch/MockSearchCluster.java
@@ -1,91 +1,25 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.search.dispatch;
 
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableMultimap;
-import com.yahoo.search.dispatch.searchcluster.Group;
+import com.yahoo.search.dispatch.searchcluster.GroupListImpl;
 import com.yahoo.search.dispatch.searchcluster.Node;
 import com.yahoo.search.dispatch.searchcluster.SearchCluster;
 import com.yahoo.vespa.config.search.DispatchConfig;
-import com.yahoo.vespa.config.search.DispatchNodesConfig;
 
-import java.util.ArrayList;
 import java.util.List;
-import java.util.Optional;
 
 /**
  * @author ollivir
  */
 public class MockSearchCluster extends SearchCluster {
 
-    private final int numGroups;
-    private final int numNodesPerGroup;
-    private final ImmutableList<Group> orderedGroups;
-    private final ImmutableMap<Integer, Group> groups;
-    private final List<Node> nodes;
-
     public MockSearchCluster(String clusterId, int groups, int nodesPerGroup) {
-        this(clusterId, createDispatchConfig(), createNodesConfig(), groups, nodesPerGroup);
+        super(clusterId, 88.0, GroupListImpl.buildGroupListForTest(groups, nodesPerGroup), null, null);
     }
 
-    public MockSearchCluster(String clusterId, DispatchConfig dispatchConfig, DispatchNodesConfig nodesConfig, int groups, int nodesPerGroup) {
-        super(clusterId, dispatchConfig, nodesConfig, null, null);
-
-        ImmutableList.Builder<Group> orderedGroupBuilder = ImmutableList.builder();
-        ImmutableMap.Builder<Integer, Group> groupBuilder = ImmutableMap.builder();
-        ImmutableMultimap.Builder<String, Node> hostBuilder = ImmutableMultimap.builder();
-        int distributionKey = 0;
-        this.nodes = new ArrayList<>();
-        for (int group = 0; group < groups; group++) {
-            List<Node> groupNodes = new ArrayList<>();
-            for (int i = 0; i < nodesPerGroup; i++) {
-                Node node = new Node(distributionKey, "host" + distributionKey, group);
-                nodes.add(node);
-                groupNodes.add(node);
-                hostBuilder.put(node.hostname(), node);
-                distributionKey++;
-            }
-            Group g = new Group(group, groupNodes);
-            groupBuilder.put(group, g);
-            orderedGroupBuilder.add(g);
-        }
-        this.orderedGroups = orderedGroupBuilder.build();
-        this.groups = groupBuilder.build();
-        this.numGroups = groups;
-        this.numNodesPerGroup = nodesPerGroup;
-    }
-
-    @Override
-    public ImmutableList<Group> orderedGroups() {
-        return orderedGroups;
-    }
-
-    @Override
-    public ImmutableMap<Integer, Group> groups() {
-        return groups;
-    }
-
-    @Override
-    public boolean allGroupsHaveSize1() { return numNodesPerGroup == 1;}
-
     @Override
     public int groupsWithSufficientCoverage() {
-        return numGroups;
-    }
-
-    @Override
-    public Optional<Group> group(int n) {
-        if (n < numGroups) {
-            return Optional.of(groups.get(n));
-        } else {
-            return Optional.empty();
-        }
-    }
-
-    @Override
-    public Optional<Node> localCorpusDispatchTarget() {
-        return Optional.empty();
+        return numGroups();
     }
 
     @Override
@@ -101,9 +35,6 @@ public class MockSearchCluster extends SearchCluster {
     public static DispatchConfig createDispatchConfig() {
         return createDispatchConfig(100.0);
     }
-    public static DispatchNodesConfig createNodesConfig(Node... nodes) {
-        return createNodesConfig(List.of(nodes)).build();
-    }
 
     public static DispatchConfig createDispatchConfig(double minSearchCoverage) {
         return createDispatchConfigBuilder(minSearchCoverage).build();
@@ -121,13 +52,4 @@ public class MockSearchCluster extends SearchCluster {
         return builder;
     }
 
-    public static DispatchNodesConfig.Builder createNodesConfig(List<Node> nodes) {
-        DispatchNodesConfig.Builder builder = new DispatchNodesConfig.Builder();
-        int port = 10000;
-        for (Node n : nodes) {
-            builder.node(new DispatchNodesConfig.Node.Builder().key(n.key()).host(n.hostname()).port(port++).group(n.group()));
-        }
-        return builder;
-    }
-
 }
diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/SearchPathTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/SearchPathTest.java
index 12cbe488485..fcd587e19f7 100644
--- a/container-search/src/test/java/com/yahoo/search/dispatch/SearchPathTest.java
+++ b/container-search/src/test/java/com/yahoo/search/dispatch/SearchPathTest.java
@@ -2,6 +2,8 @@
 package com.yahoo.search.dispatch;
 
 import com.yahoo.search.dispatch.SearchPath.InvalidSearchPathException;
+import com.yahoo.search.dispatch.searchcluster.GroupList;
+import com.yahoo.search.dispatch.searchcluster.GroupListImpl;
 import com.yahoo.search.dispatch.searchcluster.Node;
 import org.junit.jupiter.api.Test;
 
@@ -81,7 +83,7 @@ public class SearchPathTest {
         }
     }
 
-    private void verifyRandomGroup(MockSearchCluster cluster, String searchPath, Set<?> possibleSolutions) {
+    private void verifyRandomGroup(GroupList cluster, String searchPath, Set<?> possibleSolutions) {
         for (int i=0; i < 100; i++) {
             String nodes = distKeysAsString(SearchPath.selectNodes(searchPath, cluster));
             assertTrue(possibleSolutions.contains(nodes));
@@ -90,7 +92,7 @@ public class SearchPathTest {
 
     @Test
     void searchPathMustFilterNodesBasedOnDefinition() {
-        MockSearchCluster cluster = new MockSearchCluster("a", 3, 3);
+        GroupList cluster = GroupListImpl.buildGroupListForTest(3, 3);
 
         assertEquals(distKeysAsString(SearchPath.selectNodes("1/1", cluster)), "4");
         assertEquals(distKeysAsString(SearchPath.selectNodes("/1", cluster)), "3,4,5");
diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterTest.java
index 283ef29c878..ddb0b9fa809 100644
--- a/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterTest.java
+++ b/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterTest.java
@@ -7,7 +7,6 @@ import com.yahoo.container.handler.VipStatus;
 import com.yahoo.net.HostName;
 import com.yahoo.prelude.Pong;
 import com.yahoo.search.cluster.ClusterMonitor;
-import com.yahoo.search.dispatch.MockSearchCluster;
 import com.yahoo.search.result.ErrorMessage;
 import org.junit.jupiter.api.Test;
 
@@ -56,7 +55,7 @@ public class SearchClusterTest {
                 numDocsPerNode.add(new AtomicInteger(1));
                 pingCounts.add(new AtomicInteger(0));
             }
-            searchCluster = new SearchCluster(clusterId, MockSearchCluster.createDispatchConfig(), MockSearchCluster.createNodesConfig(nodes).build(),
+            searchCluster = new SearchCluster(clusterId, 100.0, nodes,
                                               vipStatus, new Factory(nodesPerGroup, numDocsPerNode, pingCounts));
             clusterMonitor = new ClusterMonitor(searchCluster, false);
             searchCluster.addMonitoring(clusterMonitor);
@@ -334,7 +333,7 @@ public class SearchClusterTest {
 
     @Test
     void requireThatEmptyGroupIsInBalance() {
-        Group group = new Group(0, new ArrayList<>());
+        Group group = new Group(0, List.of());
         assertTrue(group.isBalanced());
         group.aggregateNodeValues();
         assertTrue(group.isBalanced());
@@ -342,7 +341,7 @@ public class SearchClusterTest {
 
     @Test
     void requireThatSingleNodeGroupIsInBalance() {
-        Group group = new Group(0, Arrays.asList(new Node(1, "n", 1)));
+        Group group = new Group(0, List.of(new Node(1, "n", 1)));
         group.nodes().forEach(node -> node.setWorking(true));
         assertTrue(group.isBalanced());
         group.aggregateNodeValues();
@@ -354,7 +353,7 @@ public class SearchClusterTest {
 
     @Test
     void requireThatMultiNodeGroupDetectsBalance() {
-        Group group = new Group(0, Arrays.asList(new Node(1, "n1", 1), new Node(2, "n2", 1)));
+        Group group = new Group(0, List.of(new Node(1, "n1", 1), new Node(2, "n2", 1)));
         assertTrue(group.isBalanced());
         group.nodes().forEach(node -> node.setWorking(true));
         assertTrue(group.isBalanced());
diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterTester.java b/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterTester.java
index b5816d0c4f2..f5a9256648d 100644
--- a/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterTester.java
+++ b/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterTester.java
@@ -16,15 +16,15 @@ public class SearchClusterTester {
     }
 
     public Group group(int id) {
-        return cluster.group(id).get();
+        return cluster.group(id);
     }
 
     public void setWorking(int group, int node, boolean working) {
-        cluster.group(group).get().nodes().get(node).setWorking(working);
+        cluster.group(group).nodes().get(node).setWorking(working);
     }
 
     public void setDocsPerNode(int docs, int groupId) {
-        for (Node node : cluster.groups().get(groupId).nodes()) {
+        for (Node node : cluster.group(groupId).nodes()) {
             node.setWorking(true);
             node.setActiveDocuments(docs);
         }