Merge pull request #23653 from vespa-engine/balder/various-code-healt-and-some-extra-tests

Add some tests from an old branch and some code cleanup.

8 files changed, 136 insertions, 41 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/FilterChains.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/FilterChains.java
index bc9a45da08a..476c6249a1e 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/FilterChains.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/FilterChains.java
@@ -11,6 +11,7 @@ import com.yahoo.vespa.model.container.component.chain.Chain;
 import com.yahoo.vespa.model.container.component.chain.Chains;
 
 import java.util.Collections;
+import java.util.Set;
 
 /**
  * @author Tony Vaagenes
@@ -44,8 +45,8 @@ public class FilterChains extends Chains<Chain<Filter>>  {
     public static ChainSpecification emptyChainSpec(ComponentId chainId) {
         return new ChainSpecification(chainId,
                                       new ChainSpecification.Inheritance(null, null),
-                                      Collections.<Phase>emptySet(),
-                                      Collections.<ComponentSpecification>emptySet());
+                                      Set.of(),
+                                      Set.of());
     }
 
 }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/search/searchchain/FederationSearcher.java b/config-model/src/main/java/com/yahoo/vespa/model/container/search/searchchain/FederationSearcher.java
index 1abb62fedab..799309b8ca1 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/search/searchchain/FederationSearcher.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/search/searchchain/FederationSearcher.java
@@ -45,7 +45,7 @@ public class FederationSearcher extends Searcher<FederationSearcherModel> implem
             this.documentTypes = documentTypes;
         }
 
-        public FederationConfig.Target.SearchChain.Builder getSearchChainConfig() {
+        FederationConfig.Target.SearchChain.Builder getSearchChainConfig() {
             FederationConfig.Target.SearchChain.Builder sB = new FederationConfig.Target.SearchChain.Builder();
             FederationOptions resolvedOptions = targetOptions.inherit(searchChain.federationOptions());
             sB.
@@ -70,12 +70,12 @@ public class FederationSearcher extends Searcher<FederationSearcherModel> implem
         final ComponentId id;
         final FederationOptions targetOptions;
 
-        public Target(ComponentId id, FederationOptions targetOptions) {
+        Target(ComponentId id, FederationOptions targetOptions) {
             this.id = id;
             this.targetOptions = targetOptions;
         }
 
-        public FederationConfig.Target.Builder getTargetConfig() {
+        FederationConfig.Target.Builder getTargetConfig() {
             FederationConfig.Target.Builder tb = new FederationConfig.Target.Builder();
             tb.
                 id(id.stringValue()).
@@ -92,7 +92,7 @@ public class FederationSearcher extends Searcher<FederationSearcherModel> implem
 
         private final SearchChainConfig searchChainConfig;
 
-        public SearchChainTarget(SearchChain searchChain, FederationOptions targetOptions) {
+        SearchChainTarget(SearchChain searchChain, FederationOptions targetOptions) {
             super(searchChain.getComponentId(), targetOptions);
             searchChainConfig = new SearchChainConfig(searchChain, null, targetOptions, searchChain.getDocumentTypes());
         }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/search/searchchain/defaultsearchchains/LocalClustersCreator.java b/config-model/src/main/java/com/yahoo/vespa/model/container/search/searchchain/defaultsearchchains/LocalClustersCreator.java
index 229afd56360..0307d5d5774 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/search/searchchain/defaultsearchchains/LocalClustersCreator.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/search/searchchain/defaultsearchchains/LocalClustersCreator.java
@@ -2,15 +2,12 @@
 package com.yahoo.vespa.model.container.search.searchchain.defaultsearchchains;
 
 import com.yahoo.component.ComponentId;
-import com.yahoo.component.ComponentSpecification;
-import com.yahoo.component.chain.Phase;
 import com.yahoo.component.chain.model.ChainSpecification;
 import com.yahoo.search.searchchain.model.federation.FederationOptions;
 import com.yahoo.search.searchchain.model.federation.LocalProviderSpec;
 import com.yahoo.vespa.model.container.search.searchchain.LocalProvider;
 import com.yahoo.vespa.model.container.search.searchchain.SearchChains;
 
-import java.util.Collections;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
@@ -21,20 +18,20 @@ import java.util.Set;
  */
 public class LocalClustersCreator {
 
-    static ChainSpecification emptySearchChainSpecification(String componentName) {
+    private static ChainSpecification emptySearchChainSpecification(String componentName) {
         return new ChainSpecification(new ComponentId(componentName),
                                       VespaSearchChainsCreator.inheritsVespaPhases(), //TODO: refactor
                                       List.of(),
                                       Set.of());
     }
 
-    static LocalProvider createDefaultLocalProvider(String clusterName) {
+    private static LocalProvider createDefaultLocalProvider(String clusterName) {
         return new LocalProvider(emptySearchChainSpecification(clusterName),
                                  new FederationOptions(),
                                  new LocalProviderSpec(clusterName));
     }
 
-    static Set<String> presentClusters(SearchChains searchChains) {
+    private static Set<String> presentClusters(SearchChains searchChains) {
         Set<String> presentClusters = new LinkedHashSet<>();
         for (LocalProvider provider : searchChains.localProviders()) {
             presentClusters.add(provider.getClusterName());
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/search/searchchain/defaultsearchchains/VespaSearchChainsCreator.java b/config-model/src/main/java/com/yahoo/vespa/model/container/search/searchchain/defaultsearchchains/VespaSearchChainsCreator.java
index eb396c52fca..d9c3beaaf0e 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/search/searchchain/defaultsearchchains/VespaSearchChainsCreator.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/search/searchchain/defaultsearchchains/VespaSearchChainsCreator.java
@@ -9,10 +9,18 @@ import com.yahoo.component.chain.model.ChainedComponentModel;
 import com.yahoo.search.searchchain.PhaseNames;
 import com.yahoo.search.searchchain.model.VespaSearchers;
 import com.yahoo.search.searchchain.model.federation.FederationSearcherModel;
-import com.yahoo.vespa.model.container.component.Component;
-import com.yahoo.vespa.model.container.search.searchchain.*;
+import com.yahoo.vespa.model.container.search.searchchain.FederationSearcher;
+import com.yahoo.vespa.model.container.search.searchchain.SearchChain;
+import com.yahoo.vespa.model.container.search.searchchain.SearchChains;
+import com.yahoo.vespa.model.container.search.searchchain.Searcher;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.Optional;
 
-import java.util.*;
 
 /**
  * Creates the search chains vespaPhases, vespa and native.
@@ -36,17 +44,15 @@ public class VespaSearchChainsCreator {
             return new Phase(phase, set(before), null);
         }
 
-        public static Collection<Phase> linearPhases(String... phases) {
+        static Collection<Phase> linearPhases(String... phases) {
             List<Phase> result = new ArrayList<>();
 
             for (int i=0; i < phases.length - 1; ++i) {
-                result.add(
-                        createPhase(phases[i], phases[i+1]));
+                result.add(createPhase(phases[i], phases[i+1]));
             }
 
             if (phases.length > 0) {
-                result.add(
-                        createPhase(lastElement(phases), null));
+                result.add(createPhase(lastElement(phases), null));
             }
 
             return result;
@@ -54,11 +60,11 @@ public class VespaSearchChainsCreator {
     }
 
     private static Set<ComponentSpecification> noSearcherReferences() {
-        return Collections.emptySet();
+        return Set.of();
     }
 
     private static Collection<Phase> noPhases() {
-        return Collections.emptySet();
+        return Set.of();
     }
 
     private static ChainSpecification.Inheritance inherits(ComponentId chainId) {
@@ -79,7 +85,7 @@ public class VespaSearchChainsCreator {
 
     private static Searcher<? extends ChainedComponentModel> createSearcher(ChainedComponentModel searcherModel) {
         if (searcherModel instanceof FederationSearcherModel) {
-            return new FederationSearcher((FederationSearcherModel) searcherModel, Optional.<Component>empty());
+            return new FederationSearcher((FederationSearcherModel) searcherModel, Optional.empty());
         } else {
             return new Searcher<>(searcherModel);
         }
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/search/searchchain/FederationSearcherTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/search/searchchain/FederationSearcherTest.java
index 89319877d03..98f5bdbcd52 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/search/searchchain/FederationSearcherTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/container/search/searchchain/FederationSearcherTest.java
@@ -19,12 +19,11 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
 
-import static java.util.Collections.emptyList;
-import static java.util.Collections.emptySet;
-import static java.util.Collections.singletonList;
-import static java.util.stream.Collectors.toList;
-import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 /**
  * @author Tony Vaagenes
@@ -33,9 +32,9 @@ import static org.junit.jupiter.api.Assertions.*;
 public class FederationSearcherTest {
 
     private static class FederationFixture {
-        FederationSearcher federationSearchWithDefaultSources = newFederationSearcher(true, emptyList());
-        private ComponentRegistry<SearchChain> searchChainRegistry = new ComponentRegistry<>();
-        private SourceGroupRegistry sourceGroupRegistry = new SourceGroupRegistry();
+        FederationSearcher federationSearchWithDefaultSources = newFederationSearcher(true, List.of());
+        private final ComponentRegistry<SearchChain> searchChainRegistry = new ComponentRegistry<>();
+        private final SourceGroupRegistry sourceGroupRegistry = new SourceGroupRegistry();
 
         void initializeFederationSearcher(FederationSearcher searcher) {
             searcher.initialize(searchChainRegistry, sourceGroupRegistry);
@@ -96,14 +95,14 @@ public class FederationSearcherTest {
         assertEquals(2, target.searchChain().size());
         assertTrue(target.searchChain().stream()
                 .map(FederationConfig.Target.SearchChain::providerId)
-                .collect(toList()).containsAll(List.of("provider1", "provider2")));
+                .toList().containsAll(List.of("provider1", "provider2")));
     }
 
     @Test
     void source_groups_are_not_inherited_when_inheritDefaultSources_is_false() throws Exception {
         FederationFixture f = new ProvidersWithSourceFixture();
 
-        FederationSearcher federationSearcherWithoutDefaultSources = newFederationSearcher(false, emptyList());
+        FederationSearcher federationSearcherWithoutDefaultSources = newFederationSearcher(false, List.of());
         f.initializeFederationSearcher(federationSearcherWithoutDefaultSources);
 
         FederationConfig federationConfig = getConfig(federationSearcherWithoutDefaultSources);
@@ -127,7 +126,7 @@ public class FederationSearcherTest {
 
         f.registerProviderWithSources(createProvider(ComponentId.fromString("provider1")));
         FederationSearcher federation = newFederationSearcher(true,
-                singletonList(new TargetSpec(ComponentSpecification.fromString("provider1"),
+                List.of(new TargetSpec(ComponentSpecification.fromString("provider1"),
                         new FederationOptions().setTimeoutInMilliseconds(12345))));
         f.initializeFederationSearcher(federation);
 
@@ -147,7 +146,7 @@ public class FederationSearcherTest {
     }
 
     private static ChainSpecification searchChainSpecification(ComponentId id) {
-        return new ChainSpecification(id, new ChainSpecification.Inheritance(null, null), emptyList(), emptySet());
+        return new ChainSpecification(id, new ChainSpecification.Inheritance(null, null), List.of(), Set.of());
     }
 
     private static Provider createProvider(ComponentId id) {
@@ -161,7 +160,7 @@ public class FederationSearcherTest {
     private static FederationConfig getConfig(ConfigProducer configProducer) throws Exception {
         Optional<Class<?>> builderClassOpt = Arrays.stream(FederationConfig.class.getDeclaredClasses())
                 .filter(c -> c.getSimpleName().equals("Builder")).findFirst();
-        if (builderClassOpt.isPresent() == false) {
+        if ( builderClassOpt.isEmpty()) {
             throw new RuntimeException("No Builder class in ConfigInstance.");
         }
         Class<?> builderClass = builderClassOpt.get();
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/search/searchchain/SchemaChainsTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/search/searchchain/SchemaChainsTest.java
index 21223974be1..5588787119c 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/search/searchchain/SchemaChainsTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/container/search/searchchain/SchemaChainsTest.java
@@ -5,12 +5,17 @@ import com.yahoo.component.ComponentId;
 import com.yahoo.container.core.ChainsConfig;
 import com.yahoo.prelude.cluster.ClusterSearcher;
 import com.yahoo.search.config.ClusterConfig;
+import com.yahoo.search.searchchain.PhaseNames;
 import com.yahoo.vespa.defaults.Defaults;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.w3c.dom.Element;
 
+import java.util.Collection;
+
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 
 /**
@@ -21,10 +26,15 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
  */
 public class SchemaChainsTest extends SchemaChainsTestBase {
 
+    private ChainsConfig chainsConfig;
     private ClusterConfig clusterConfig;
 
     @BeforeEach
     public void subscribe() {
+        ChainsConfig.Builder chainsBuilder = new ChainsConfig.Builder();
+        chainsBuilder = (ChainsConfig.Builder)root.getConfig(chainsBuilder, "searchchains");
+        chainsConfig = new ChainsConfig(chainsBuilder);
+
         ClusterConfig.Builder clusterBuilder = new ClusterConfig.Builder();
         clusterBuilder = (ClusterConfig.Builder)root.getConfig(clusterBuilder, "searchchains/chain/cluster2/component/" + ClusterSearcher.class.getName());
         clusterConfig = new ClusterConfig(clusterBuilder);
@@ -81,4 +91,79 @@ public class SchemaChainsTest extends SchemaChainsTestBase {
         assertEquals("cluster2", clusterConfig.clusterName());
     }
 
+    private ChainsConfig.Chains findChain(String name) {
+        for (ChainsConfig.Chains chain : chainsConfig.chains()) {
+            if (name.equals(chain.id())) {
+                return chain;
+            }
+        }
+        return null;
+    }
+
+    private static boolean contains(Collection<ChainsConfig.Chains.Phases> phases, String name) {
+        for (var phase : phases) {
+            if (name.equals(phase.id())) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    private static void validateVespaPhasesChain(ChainsConfig.Chains chain) {
+        assertNotNull(chain);
+        assertEquals("vespaPhases", chain.id());
+        assertEquals(5, chain.phases().size());
+        assertTrue(contains(chain.phases(), PhaseNames.BACKEND));
+        assertTrue(contains(chain.phases(), PhaseNames.BLENDED_RESULT));
+        assertTrue(contains(chain.phases(), PhaseNames.RAW_QUERY));
+        assertTrue(contains(chain.phases(), PhaseNames.TRANSFORMED_QUERY));
+        assertTrue(contains(chain.phases(), PhaseNames.UNBLENDED_RESULT));
+        assertTrue(chain.inherits().isEmpty());
+        assertTrue(chain.components().isEmpty());
+        assertTrue(chain.excludes().isEmpty());
+        assertEquals(ChainsConfig.Chains.Type.SEARCH, chain.type());
+    }
+
+    private static void validateNativeChain(ChainsConfig.Chains chain) {
+        assertNotNull(chain);
+        assertEquals("native", chain.id());
+        assertTrue(chain.phases().isEmpty());
+        assertEquals(1, chain.inherits().size());
+        assertEquals("vespaPhases", chain.inherits(0));
+        assertEquals(2, chain.components().size());
+        assertEquals("federation@native", chain.components(0));
+        assertEquals("com.yahoo.prelude.statistics.StatisticsSearcher@native", chain.components(1));
+        assertTrue(chain.excludes().isEmpty());
+        assertEquals(ChainsConfig.Chains.Type.SEARCH, chain.type());
+    }
+
+    private static void validateVespaChain(ChainsConfig.Chains chain) {
+        assertNotNull(chain);
+        assertEquals("vespa", chain.id());
+        assertTrue(chain.phases().isEmpty());
+        assertEquals(1, chain.inherits().size());
+        assertEquals("native", chain.inherits(0));
+        assertEquals(10, chain.components().size());
+        assertEquals("com.yahoo.prelude.querytransform.PhrasingSearcher@vespa", chain.components(0));
+        assertEquals("com.yahoo.prelude.searcher.FieldCollapsingSearcher@vespa", chain.components(1));
+        assertEquals("com.yahoo.search.yql.MinimalQueryInserter@vespa", chain.components(2));
+        assertEquals("com.yahoo.search.yql.FieldFilter@vespa", chain.components(3));
+        assertEquals("com.yahoo.prelude.searcher.JuniperSearcher@vespa", chain.components(4));
+        assertEquals("com.yahoo.prelude.searcher.BlendingSearcher@vespa", chain.components(5));
+        assertEquals("com.yahoo.prelude.searcher.PosSearcher@vespa", chain.components(6));
+        assertEquals("com.yahoo.prelude.semantics.SemanticSearcher@vespa", chain.components(7));
+        assertEquals("com.yahoo.search.grouping.GroupingQueryParser@vespa", chain.components(8));
+        assertEquals("com.yahoo.search.querytransform.WeakAndReplacementSearcher@vespa", chain.components(9));
+        assertTrue(chain.excludes().isEmpty());
+        assertEquals(ChainsConfig.Chains.Type.SEARCH, chain.type());
+    }
+
+    @Test
+    public void require_all_default_chains_are_correct() {
+        assertEquals(61, chainsConfig.components().size());
+        assertEquals(10, chainsConfig.chains().size());
+        validateVespaPhasesChain(findChain("vespaPhases"));
+        validateNativeChain(findChain("native"));
+        validateVespaChain(findChain("vespa"));
+    }
 }
diff --git a/container-core/src/main/java/com/yahoo/component/chain/model/ChainSpecification.java b/container-core/src/main/java/com/yahoo/component/chain/model/ChainSpecification.java
index a1c169dc387..f53b431265c 100644
--- a/container-core/src/main/java/com/yahoo/component/chain/model/ChainSpecification.java
+++ b/container-core/src/main/java/com/yahoo/component/chain/model/ChainSpecification.java
@@ -6,7 +6,15 @@ import com.yahoo.component.ComponentId;
 import com.yahoo.component.ComponentSpecification;
 import com.yahoo.component.chain.Phase;
 
-import java.util.*;
+import java.util.ArrayDeque;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Deque;
+import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
+import java.util.Map;
+import java.util.Set;
+
 
 /**
  * Specifies how the components should be selected to create a chain. Immutable.
@@ -20,7 +28,7 @@ public class ChainSpecification {
         public final Set<ComponentSpecification> excludedComponents;
 
         Inheritance flattened() {
-            return new Inheritance(Collections.<ComponentSpecification>emptySet(), excludedComponents);
+            return new Inheritance(Set.of(), excludedComponents);
         }
 
         public Inheritance(Set<ComponentSpecification> inheritedChains, Set<ComponentSpecification> excludedComponents) {
@@ -38,7 +46,7 @@ public class ChainSpecification {
 
     public final ComponentId componentId;
     public final Inheritance inheritance;
-    final Map<String, Phase> phases;
+    private final Map<String, Phase> phases;
     public final Set<ComponentSpecification> componentReferences;
 
     public ChainSpecification(ComponentId componentId, Inheritance inheritance,
diff --git a/container-search/src/test/java/com/yahoo/search/searchchain/test/SearchChainTestCase.java b/container-search/src/test/java/com/yahoo/search/searchchain/test/SearchChainTestCase.java
index 3e7a152122e..ef35b5beabe 100644
--- a/container-search/src/test/java/com/yahoo/search/searchchain/test/SearchChainTestCase.java
+++ b/container-search/src/test/java/com/yahoo/search/searchchain/test/SearchChainTestCase.java
@@ -26,7 +26,6 @@ import org.junit.jupiter.api.Test;
  *
  * @author bratseth
  */
-@SuppressWarnings("deprecation")
 public class SearchChainTestCase {
 
     @Test
@@ -87,7 +86,7 @@ public class SearchChainTestCase {
     private List<Searcher> createSearchers(int count) {
         List<Searcher> searchers=new ArrayList<>(count);
         for (int i=0; i<count; i++)
-            searchers.add(new TestSearcher("s" + String.valueOf(i+1)));
+            searchers.add(new TestSearcher("s" + (i+1)));
         return searchers;
     }