From 1b5eeb6a730364586dfe7b438140269b2352f36c Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Thu, 18 Apr 2024 13:14:04 +0200 Subject: Move Results to the only place it is used. --- .../search/federation/FederationSearcher.java | 11 ++-- .../java/com/yahoo/search/federation/Results.java | 50 +++++++++++++++++++ .../federation/sourceref/SearchChainResolver.java | 10 ---- .../main/java/com/yahoo/errorhandling/Results.java | 58 ---------------------- .../java/com/yahoo/errorhandling/package-info.java | 5 -- 5 files changed, 55 insertions(+), 79 deletions(-) create mode 100644 container-search/src/main/java/com/yahoo/search/federation/Results.java delete mode 100644 vespajlib/src/main/java/com/yahoo/errorhandling/Results.java delete mode 100644 vespajlib/src/main/java/com/yahoo/errorhandling/package-info.java diff --git a/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java b/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java index 72184c5ea32..8aca45ef900 100644 --- a/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java +++ b/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java @@ -9,8 +9,7 @@ import com.yahoo.component.chain.Chain; import com.yahoo.component.chain.dependencies.After; import com.yahoo.component.chain.dependencies.Provides; import com.yahoo.component.provider.ComponentRegistry; -import com.yahoo.errorhandling.Results; -import com.yahoo.errorhandling.Results.Builder; +import com.yahoo.search.federation.Results.Builder; import com.yahoo.processing.IllegalInputException; import com.yahoo.processing.request.CompoundName; import com.yahoo.search.Query; @@ -88,7 +87,7 @@ public class FederationSearcher extends ForkingSearcher { @Inject public FederationSearcher(FederationConfig config, SchemaInfo schemaInfo, - ComponentRegistry targetSelectors) { + ComponentRegistry> targetSelectors) { this(createResolver(config), createVirtualSourceResolver(config), resolveSelector(config.targetSelector(), targetSelectors), @@ -103,7 +102,7 @@ public class FederationSearcher extends ForkingSearcher { private FederationSearcher(SearchChainResolver searchChainResolver, VirtualSourceResolver virtualSourceResolver, - TargetSelector targetSelector, + TargetSelector targetSelector, Map> schema2Clusters) { this.searchChainResolver = searchChainResolver; sourceRefResolver = new SourceRefResolver(searchChainResolver, schema2Clusters); @@ -115,8 +114,8 @@ public class FederationSearcher extends ForkingSearcher { return VirtualSourceResolver.of(config.target().stream().map(FederationConfig.Target::id).collect(Collectors.toUnmodifiableSet())); } - private static TargetSelector resolveSelector(String selectorId, - ComponentRegistry targetSelectors) { + private static TargetSelector resolveSelector(String selectorId, + ComponentRegistry> targetSelectors) { if (selectorId.isEmpty()) return null; return checkNotNull(targetSelectors.getComponent(selectorId), "Missing target selector with id '" + selectorId + "'"); diff --git a/container-search/src/main/java/com/yahoo/search/federation/Results.java b/container-search/src/main/java/com/yahoo/search/federation/Results.java new file mode 100644 index 00000000000..b27769413e3 --- /dev/null +++ b/container-search/src/main/java/com/yahoo/search/federation/Results.java @@ -0,0 +1,50 @@ +// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.search.federation; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; + +/** + * @author Tony Vaagenes + */ +public class Results { + + private final List data; + private final List errors; + + private Results(List data, List errors) { + this.data = List.copyOf(data); + this.errors = List.copyOf(errors); + } + + public List data() { + return data; + } + + public List errors() { + return errors; + } + + public static class Builder { + private final List data = new ArrayList<>(); + private final List errors = new ArrayList<>(); + + public void addData(DATA d) { + data.add(d); + } + + public void addAllData(Collection d) { + data.addAll(d); + } + + public void addError(ERROR e) { + errors.add(e); + } + + public Results build() { + return new Results<>(data, errors); + } + } + +} diff --git a/container-search/src/main/java/com/yahoo/search/federation/sourceref/SearchChainResolver.java b/container-search/src/main/java/com/yahoo/search/federation/sourceref/SearchChainResolver.java index 7dc65c819e4..5101c08a265 100644 --- a/container-search/src/main/java/com/yahoo/search/federation/sourceref/SearchChainResolver.java +++ b/container-search/src/main/java/com/yahoo/search/federation/sourceref/SearchChainResolver.java @@ -46,16 +46,6 @@ public class SearchChainResolver { public static class Builder { - public interface InvocationSpecFactory { - SearchChainInvocationSpec create(ComponentId searchChainId, FederationOptions federationOptions, List schemas); - } - - private class DefaultInvocationSpecFactory implements InvocationSpecFactory { - public SearchChainInvocationSpec create(ComponentId searchChainId, FederationOptions federationOptions, List schemas) { - return new SearchChainInvocationSpec(searchChainId, federationOptions, schemas); - } - } - private final SortedSet defaultTargets = new TreeSet<>(); private final ComponentRegistry targets = new ComponentRegistry<>() { diff --git a/vespajlib/src/main/java/com/yahoo/errorhandling/Results.java b/vespajlib/src/main/java/com/yahoo/errorhandling/Results.java deleted file mode 100644 index 939d2276efc..00000000000 --- a/vespajlib/src/main/java/com/yahoo/errorhandling/Results.java +++ /dev/null @@ -1,58 +0,0 @@ -// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.errorhandling; - -import java.util.ArrayList; -import java.util.Collection; -import java.util.List; - -/** - * @author Tony Vaagenes - */ -public class Results { - - private final List data; - private final List errors; - - public Results(List data, List errors) { - this.data = List.copyOf(data); - this.errors = List.copyOf(errors); - } - - public boolean hasErrors() { - return !errors.isEmpty(); - } - - public List data() { - return data; - } - - public List errors() { - return errors; - } - - public static class Builder { - private final List data = new ArrayList<>(); - private final List errors = new ArrayList<>(); - - public void addData(DATA d) { - data.add(d); - } - - public void addAllData(Collection d) { - data.addAll(d); - } - - public void addError(ERROR e) { - errors.add(e); - } - - public void addAllErrors(Collection e) { - errors.addAll(e); - } - - public Results build() { - return new Results<>(data, errors); - } - } - -} diff --git a/vespajlib/src/main/java/com/yahoo/errorhandling/package-info.java b/vespajlib/src/main/java/com/yahoo/errorhandling/package-info.java deleted file mode 100644 index ac6c913381c..00000000000 --- a/vespajlib/src/main/java/com/yahoo/errorhandling/package-info.java +++ /dev/null @@ -1,5 +0,0 @@ -// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -@ExportPackage -package com.yahoo.errorhandling; - -import com.yahoo.osgi.annotation.ExportPackage; -- cgit v1.2.3 From d063f6716eea6c6eaa5623f95a3d1c1b2af408da Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Thu, 18 Apr 2024 23:13:41 +0200 Subject: Use --- .../test/java/com/yahoo/search/federation/FederationSearcherTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/container-search/src/test/java/com/yahoo/search/federation/FederationSearcherTest.java b/container-search/src/test/java/com/yahoo/search/federation/FederationSearcherTest.java index 1ac6dcbe1b6..b46457cf6b3 100644 --- a/container-search/src/test/java/com/yahoo/search/federation/FederationSearcherTest.java +++ b/container-search/src/test/java/com/yahoo/search/federation/FederationSearcherTest.java @@ -163,7 +163,7 @@ public class FederationSearcherTest { @Test void custom_federation_target() { ComponentId targetSelectorId = ComponentId.fromString("TargetSelector"); - ComponentRegistry targetSelectors = new ComponentRegistry<>(); + ComponentRegistry> targetSelectors = new ComponentRegistry<>(); targetSelectors.register(targetSelectorId, new TestTargetSelector()); FederationSearcher searcher = new FederationSearcher( @@ -182,7 +182,7 @@ public class FederationSearcherTest { @Test void target_selectors_can_have_multiple_targets() { ComponentId targetSelectorId = ComponentId.fromString("TestMultipleTargetSelector"); - ComponentRegistry targetSelectors = new ComponentRegistry<>(); + ComponentRegistry> targetSelectors = new ComponentRegistry<>(); targetSelectors.register(targetSelectorId, new TestMultipleTargetSelector()); FederationSearcher searcher = new FederationSearcher( -- cgit v1.2.3 From adca169ef5243549fd9f811511eb519898ffecf8 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Fri, 19 Apr 2024 14:08:54 +0200 Subject: Revert . Can not be used with components and injection. --- .../main/java/com/yahoo/search/federation/FederationSearcher.java | 8 ++++---- .../java/com/yahoo/search/federation/FederationSearcherTest.java | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java b/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java index 8aca45ef900..b2f907c1e89 100644 --- a/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java +++ b/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java @@ -87,7 +87,7 @@ public class FederationSearcher extends ForkingSearcher { @Inject public FederationSearcher(FederationConfig config, SchemaInfo schemaInfo, - ComponentRegistry> targetSelectors) { + ComponentRegistry targetSelectors) { this(createResolver(config), createVirtualSourceResolver(config), resolveSelector(config.targetSelector(), targetSelectors), @@ -102,7 +102,7 @@ public class FederationSearcher extends ForkingSearcher { private FederationSearcher(SearchChainResolver searchChainResolver, VirtualSourceResolver virtualSourceResolver, - TargetSelector targetSelector, + TargetSelector targetSelector, Map> schema2Clusters) { this.searchChainResolver = searchChainResolver; sourceRefResolver = new SourceRefResolver(searchChainResolver, schema2Clusters); @@ -114,8 +114,8 @@ public class FederationSearcher extends ForkingSearcher { return VirtualSourceResolver.of(config.target().stream().map(FederationConfig.Target::id).collect(Collectors.toUnmodifiableSet())); } - private static TargetSelector resolveSelector(String selectorId, - ComponentRegistry> targetSelectors) { + private static TargetSelector resolveSelector(String selectorId, + ComponentRegistry targetSelectors) { if (selectorId.isEmpty()) return null; return checkNotNull(targetSelectors.getComponent(selectorId), "Missing target selector with id '" + selectorId + "'"); diff --git a/container-search/src/test/java/com/yahoo/search/federation/FederationSearcherTest.java b/container-search/src/test/java/com/yahoo/search/federation/FederationSearcherTest.java index b46457cf6b3..1ac6dcbe1b6 100644 --- a/container-search/src/test/java/com/yahoo/search/federation/FederationSearcherTest.java +++ b/container-search/src/test/java/com/yahoo/search/federation/FederationSearcherTest.java @@ -163,7 +163,7 @@ public class FederationSearcherTest { @Test void custom_federation_target() { ComponentId targetSelectorId = ComponentId.fromString("TargetSelector"); - ComponentRegistry> targetSelectors = new ComponentRegistry<>(); + ComponentRegistry targetSelectors = new ComponentRegistry<>(); targetSelectors.register(targetSelectorId, new TestTargetSelector()); FederationSearcher searcher = new FederationSearcher( @@ -182,7 +182,7 @@ public class FederationSearcherTest { @Test void target_selectors_can_have_multiple_targets() { ComponentId targetSelectorId = ComponentId.fromString("TestMultipleTargetSelector"); - ComponentRegistry> targetSelectors = new ComponentRegistry<>(); + ComponentRegistry targetSelectors = new ComponentRegistry<>(); targetSelectors.register(targetSelectorId, new TestMultipleTargetSelector()); FederationSearcher searcher = new FederationSearcher( -- cgit v1.2.3 From de6974a1a42743fd137b32122b03ec1b48c469a5 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Fri, 19 Apr 2024 20:19:22 +0200 Subject: Aviod using exception for normal control flow. --- .../search/federation/FederationSearcher.java | 93 ++++++++++++---------- .../java/com/yahoo/search/federation/Results.java | 5 -- .../search/federation/sourceref/ResolveResult.java | 14 ++++ .../federation/sourceref/SearchChainResolver.java | 12 +-- .../search/federation/sourceref/SingleTarget.java | 4 +- .../federation/sourceref/SourceRefResolver.java | 43 +++++----- .../search/federation/sourceref/SourcesTarget.java | 19 ++--- .../yahoo/search/federation/sourceref/Target.java | 3 +- .../sourceref/UnresolvedProviderException.java | 20 ----- .../sourceref/UnresolvedSearchChainException.java | 12 --- .../sourceref/UnresolvedSourceRefException.java | 18 ----- .../sourceref/SearchChainResolverTestCase.java | 46 +++++------ .../sourceref/SourceRefResolverTestCase.java | 42 +++------- 13 files changed, 133 insertions(+), 198 deletions(-) create mode 100644 container-search/src/main/java/com/yahoo/search/federation/sourceref/ResolveResult.java delete mode 100644 container-search/src/main/java/com/yahoo/search/federation/sourceref/UnresolvedProviderException.java delete mode 100644 container-search/src/main/java/com/yahoo/search/federation/sourceref/UnresolvedSearchChainException.java delete mode 100644 container-search/src/main/java/com/yahoo/search/federation/sourceref/UnresolvedSourceRefException.java diff --git a/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java b/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java index b2f907c1e89..4d9111b2711 100644 --- a/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java +++ b/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java @@ -9,7 +9,6 @@ import com.yahoo.component.chain.Chain; import com.yahoo.component.chain.dependencies.After; import com.yahoo.component.chain.dependencies.Provides; import com.yahoo.component.provider.ComponentRegistry; -import com.yahoo.search.federation.Results.Builder; import com.yahoo.processing.IllegalInputException; import com.yahoo.processing.request.CompoundName; import com.yahoo.search.Query; @@ -18,12 +17,12 @@ import com.yahoo.search.Searcher; import com.yahoo.search.federation.selection.FederationTarget; import com.yahoo.search.federation.selection.TargetSelector; import com.yahoo.search.federation.sourceref.ModifyQueryAndResult; +import com.yahoo.search.federation.sourceref.ResolveResult; import com.yahoo.search.federation.sourceref.SearchChainInvocationSpec; import com.yahoo.search.federation.sourceref.SearchChainResolver; import com.yahoo.search.federation.sourceref.SingleTarget; import com.yahoo.search.federation.sourceref.SourceRefResolver; import com.yahoo.search.federation.sourceref.SourcesTarget; -import com.yahoo.search.federation.sourceref.UnresolvedSearchChainException; import com.yahoo.search.federation.sourceref.VirtualSourceResolver; import com.yahoo.search.query.Properties; import com.yahoo.search.result.ErrorMessage; @@ -52,6 +51,7 @@ import java.util.Map.Entry; import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.TreeSet; import java.util.concurrent.TimeUnit; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -205,14 +205,44 @@ public class FederationSearcher extends ForkingSearcher { setRequestTimeoutInMilliseconds(searchChain.requestTimeoutMillis()); } + private static List extractErrors(List results) { + List errors = List.of(); + for (ResolveResult result : results) { + if (result.errorMsg() != null) { + if (errors.isEmpty()) { + errors = new ArrayList<>(); + } + errors.add(result.errorMsg()); + } + } + return errors; + } + + private static List extractSpecs(List results) { + List errors = List.of(); + for (ResolveResult result : results) { + if (result.invocationSpec() != null) { + if (errors.isEmpty()) { + errors = List.of(result.invocationSpec()); + } else if (errors.size() == 1) { + errors = new ArrayList<>(errors); + errors.add(result.invocationSpec()); + } else { + errors.add(result.invocationSpec()); + } + } + } + return errors; + } + @Override public Result search(Query query, Execution execution) { Result mergedResults = execution.search(query); var targets = getTargets(query.getModel().getSources(), query.properties()); - warnIfUnresolvedSearchChains(targets.errors(), mergedResults.hits()); + warnIfUnresolvedSearchChains(extractErrors(targets), mergedResults.hits()); - var prunedTargets = pruneTargetsWithoutDocumentTypes(query.getModel().getRestrict(), targets.data()); + var prunedTargets = pruneTargetsWithoutDocumentTypes(query.getModel().getRestrict(), extractSpecs(targets)); var regularTargetHandlers = resolveSearchChains(prunedTargets, execution.searchChainRegistry()); query.errors().addAll(regularTargetHandlers.errors()); @@ -310,32 +340,19 @@ public class FederationSearcher extends ForkingSearcher { .forEach((k, v) -> outgoing.properties().set(k, v)); } - private ErrorMessage missingSearchChainsErrorMessage(List unresolvedSearchChainExceptions) { - String message = String.join(" ", getMessagesSet(unresolvedSearchChainExceptions)) + + private ErrorMessage missingSearchChainsErrorMessage(List errors) { + String message = String.join(" ", new TreeSet<>(errors)) + " Valid source refs are " + String.join(", ", allSourceRefDescriptions()) +'.'; return ErrorMessage.createInvalidQueryParameter(message); } private List allSourceRefDescriptions() { - List descriptions = new ArrayList<>(); - - for (com.yahoo.search.federation.sourceref.Target target : searchChainResolver.allTopLevelTargets()) - descriptions.add(target.searchRefDescription()); - return descriptions; - } - - private static Set getMessagesSet(List unresolvedSearchChainExceptions) { - Set messages = new LinkedHashSet<>(); - for (UnresolvedSearchChainException exception : unresolvedSearchChainExceptions) { - messages.add(exception.getMessage()); - } - return messages; + return searchChainResolver.allTopLevelTargets().stream().map(com.yahoo.search.federation.sourceref.Target::searchRefDescription).toList(); } - private void warnIfUnresolvedSearchChains(List missingTargets, - HitGroup errorHitGroup) { - if (!missingTargets.isEmpty()) { - errorHitGroup.addError(missingSearchChainsErrorMessage(missingTargets)); + private void warnIfUnresolvedSearchChains(List errorMessages, HitGroup errorHitGroup) { + if (!errorMessages.isEmpty()) { + errorHitGroup.addError(missingSearchChainsErrorMessage(errorMessages)); } } @@ -343,7 +360,7 @@ public class FederationSearcher extends ForkingSearcher { public Collection getSearchChainsForwarded(SearchChainRegistry registry) { List searchChains = new ArrayList<>(); - for (com.yahoo.search.federation.sourceref.Target target : searchChainResolver.allTopLevelTargets()) { + for (var target : searchChainResolver.allTopLevelTargets()) { if (target instanceof SourcesTarget) { searchChains.addAll(commentedSourceProviderSearchChains((SourcesTarget)target, registry)); } else if (target instanceof SingleTarget) { @@ -467,40 +484,32 @@ public class FederationSearcher extends ForkingSearcher { return orderer; } - private Results getTargets(Set sources, Properties properties) { + private List getTargets(Set sources, Properties properties) { return sources.isEmpty() ? defaultSearchChains(properties): resolveSources(sources, properties); } - private Results resolveSources(Set sourcesInQuery, Properties properties) { - Results.Builder result = new Builder<>(); + private List resolveSources(Set sourcesInQuery, Properties properties) { + List result = new ArrayList<>(); Set sources = virtualSourceResolver.resolve(sourcesInQuery); for (String source : sources) { - try { - result.addAllData(sourceRefResolver.resolve(asSourceSpec(source), properties)); - } catch (UnresolvedSearchChainException e) { - result.addError(e); - } + result.addAll(sourceRefResolver.resolve(asSourceSpec(source), properties)); } - return result.build(); + return List.copyOf(result); } - public Results defaultSearchChains(Properties sourceToProviderMap) { - Results.Builder result = new Builder<>(); + public List defaultSearchChains(Properties sourceToProviderMap) { + List result = new ArrayList<>(); - for (com.yahoo.search.federation.sourceref.Target target : searchChainResolver.defaultTargets()) { - try { - result.addData(target.responsibleSearchChain(sourceToProviderMap)); - } catch (UnresolvedSearchChainException e) { - result.addError(e); - } + for (var target : searchChainResolver.defaultTargets()) { + result.add(target.responsibleSearchChain(sourceToProviderMap)); } - return result.build(); + return List.copyOf(result); } diff --git a/container-search/src/main/java/com/yahoo/search/federation/Results.java b/container-search/src/main/java/com/yahoo/search/federation/Results.java index b27769413e3..7598a14f759 100644 --- a/container-search/src/main/java/com/yahoo/search/federation/Results.java +++ b/container-search/src/main/java/com/yahoo/search/federation/Results.java @@ -33,11 +33,6 @@ public class Results { public void addData(DATA d) { data.add(d); } - - public void addAllData(Collection d) { - data.addAll(d); - } - public void addError(ERROR e) { errors.add(e); } diff --git a/container-search/src/main/java/com/yahoo/search/federation/sourceref/ResolveResult.java b/container-search/src/main/java/com/yahoo/search/federation/sourceref/ResolveResult.java new file mode 100644 index 00000000000..d9681140ae9 --- /dev/null +++ b/container-search/src/main/java/com/yahoo/search/federation/sourceref/ResolveResult.java @@ -0,0 +1,14 @@ +// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.search.federation.sourceref; + +/** + * @author baldersheim + */ +public record ResolveResult(SearchChainInvocationSpec invocationSpec, String errorMsg) { + ResolveResult(SearchChainInvocationSpec invocationSpec) { + this(invocationSpec, null); + } + ResolveResult(String errorMsg) { + this(null, errorMsg); + } +} diff --git a/container-search/src/main/java/com/yahoo/search/federation/sourceref/SearchChainResolver.java b/container-search/src/main/java/com/yahoo/search/federation/sourceref/SearchChainResolver.java index 5101c08a265..9e45b6576a6 100644 --- a/container-search/src/main/java/com/yahoo/search/federation/sourceref/SearchChainResolver.java +++ b/container-search/src/main/java/com/yahoo/search/federation/sourceref/SearchChainResolver.java @@ -127,19 +127,13 @@ public class SearchChainResolver { this.defaultTargets = Collections.unmodifiableSortedSet(defaultTargets); } - public SearchChainInvocationSpec resolve(ComponentSpecification sourceRef, Properties sourceToProviderMap) - throws UnresolvedSearchChainException { + public ResolveResult resolve(ComponentSpecification sourceRef, Properties sourceToProviderMap) { - Target target = resolveTarget(sourceRef); - return target.responsibleSearchChain(sourceToProviderMap); - } - - private Target resolveTarget(ComponentSpecification sourceRef) throws UnresolvedSearchChainException { Target target = targets.getComponent(sourceRef); if (target == null) { - throw UnresolvedSourceRefException.createForMissingSourceRef(sourceRef); + return new ResolveResult(SourceRefResolver.createForMissingSourceRef(sourceRef)); } - return target; + return target.responsibleSearchChain(sourceToProviderMap); } public SortedSet allTopLevelTargets() { diff --git a/container-search/src/main/java/com/yahoo/search/federation/sourceref/SingleTarget.java b/container-search/src/main/java/com/yahoo/search/federation/sourceref/SingleTarget.java index 608566552cd..3de67908217 100644 --- a/container-search/src/main/java/com/yahoo/search/federation/sourceref/SingleTarget.java +++ b/container-search/src/main/java/com/yahoo/search/federation/sourceref/SingleTarget.java @@ -17,8 +17,8 @@ public class SingleTarget extends Target { } @Override - public SearchChainInvocationSpec responsibleSearchChain(Properties queryProperties) { - return searchChainInvocationSpec; + public ResolveResult responsibleSearchChain(Properties queryProperties) { + return new ResolveResult(searchChainInvocationSpec); } @Override diff --git a/container-search/src/main/java/com/yahoo/search/federation/sourceref/SourceRefResolver.java b/container-search/src/main/java/com/yahoo/search/federation/sourceref/SourceRefResolver.java index 2e7849dd85a..b5c40db01f8 100644 --- a/container-search/src/main/java/com/yahoo/search/federation/sourceref/SourceRefResolver.java +++ b/container-search/src/main/java/com/yahoo/search/federation/sourceref/SourceRefResolver.java @@ -4,10 +4,9 @@ package com.yahoo.search.federation.sourceref; import com.yahoo.component.ComponentSpecification; import com.yahoo.processing.request.Properties; -import java.util.LinkedHashSet; +import java.util.ArrayList; import java.util.List; import java.util.Map; -import java.util.Set; /** * Maps a source reference to search chain invocation specs. @@ -24,21 +23,18 @@ public class SourceRefResolver { this.schema2Clusters = schema2Clusters; } - public Set resolve(ComponentSpecification sourceRef, - Properties sourceToProviderMap) throws UnresolvedSearchChainException { - try { - return Set.of(searchChainResolver.resolve(sourceRef, sourceToProviderMap)); - } catch (UnresolvedSourceRefException e) { + public List resolve(ComponentSpecification sourceRef, Properties sourceToProviderMap) { + ResolveResult searchChainResolveResult = searchChainResolver.resolve(sourceRef, sourceToProviderMap); + if (searchChainResolveResult.invocationSpec() == null) { return resolveClustersWithDocument(sourceRef, sourceToProviderMap); } + return List.of(searchChainResolveResult); } - private Set resolveClustersWithDocument(ComponentSpecification sourceRef, - Properties sourceToProviderMap) - throws UnresolvedSearchChainException { + private List resolveClustersWithDocument(ComponentSpecification sourceRef, Properties sourceToProviderMap) { if (hasOnlyName(sourceRef)) { - Set clusterSearchChains = new LinkedHashSet<>(); + List clusterSearchChains = new ArrayList<>(); List clusters = schema2Clusters.getOrDefault(sourceRef.getName(), List.of()); for (String cluster : clusters) { @@ -48,21 +44,22 @@ public class SourceRefResolver { if ( ! clusterSearchChains.isEmpty()) return clusterSearchChains; } - throw UnresolvedSourceRefException.createForMissingSourceRef(sourceRef); + return List.of(new ResolveResult(createForMissingSourceRef(sourceRef))); } - private SearchChainInvocationSpec resolveClusterSearchChain(String cluster, - ComponentSpecification sourceRef, - Properties sourceToProviderMap) - throws UnresolvedSearchChainException { - try { - return searchChainResolver.resolve(new ComponentSpecification(cluster), sourceToProviderMap); - } - catch (UnresolvedSearchChainException e) { - throw new UnresolvedSearchChainException("Failed to resolve cluster search chain '" + cluster + - "' when using source ref '" + sourceRef + - "' as a document name."); + static String createForMissingSourceRef(ComponentSpecification source) { + return "Could not resolve source ref '" + source + "'."; + } + + private ResolveResult resolveClusterSearchChain(String cluster, + ComponentSpecification sourceRef, + Properties sourceToProviderMap) { + var resolveResult = searchChainResolver.resolve(new ComponentSpecification(cluster), sourceToProviderMap); + if (resolveResult.invocationSpec() == null) { + return new ResolveResult("Failed to resolve cluster search chain '" + cluster + + "' when using source ref '" + sourceRef + "' as a document name."); } + return resolveResult; } private boolean hasOnlyName(ComponentSpecification sourceSpec) { diff --git a/container-search/src/main/java/com/yahoo/search/federation/sourceref/SourcesTarget.java b/container-search/src/main/java/com/yahoo/search/federation/sourceref/SourcesTarget.java index b6d99758c7b..a3c0328290d 100644 --- a/container-search/src/main/java/com/yahoo/search/federation/sourceref/SourcesTarget.java +++ b/container-search/src/main/java/com/yahoo/search/federation/sourceref/SourcesTarget.java @@ -16,7 +16,7 @@ import java.util.TreeSet; public class SourcesTarget extends Target { - private ComponentRegistry> providerSources = new ComponentRegistry<>() {}; + private final ComponentRegistry> providerSources = new ComponentRegistry<>() {}; private SearchChainInvocationSpec defaultProviderSource; @@ -25,10 +25,10 @@ public class SourcesTarget extends Target { } @Override - public SearchChainInvocationSpec responsibleSearchChain(Properties queryProperties) throws UnresolvedSearchChainException { + public ResolveResult responsibleSearchChain(Properties queryProperties) { ComponentSpecification providerSpecification = providerSpecificationForSource(queryProperties); if (providerSpecification == null) { - return defaultProviderSource; + return new ResolveResult(defaultProviderSource); } else { return lookupProviderSource(providerSpecification); } @@ -36,11 +36,7 @@ public class SourcesTarget extends Target { @Override public String searchRefDescription() { - StringBuilder builder = new StringBuilder(sourceId().stringValue()); - builder.append("[provider = "). - append(Joiner.on(", ").join(allProviderIdsStringValue())). - append("]"); - return builder.toString(); + return sourceId().stringValue() + "[provider = " + Joiner.on(", ").join(allProviderIdsStringValue()) + "]"; } private SortedSet allProviderIdsStringValue() { @@ -51,14 +47,13 @@ public class SourcesTarget extends Target { return result; } - private SearchChainInvocationSpec lookupProviderSource(ComponentSpecification providerSpecification) - throws UnresolvedSearchChainException { + private ResolveResult lookupProviderSource(ComponentSpecification providerSpecification) { ComponentAdaptor providerSource = providerSources.getComponent(providerSpecification); if (providerSource == null) - throw UnresolvedProviderException.createForMissingProvider(sourceId(), providerSpecification); + return new ResolveResult("No provider '" + sourceId() + "' for source '" + providerSpecification + "'."); - return providerSource.model; + return new ResolveResult(providerSource.model); } public void freeze() { diff --git a/container-search/src/main/java/com/yahoo/search/federation/sourceref/Target.java b/container-search/src/main/java/com/yahoo/search/federation/sourceref/Target.java index 38baf084d97..d35f7f7b181 100644 --- a/container-search/src/main/java/com/yahoo/search/federation/sourceref/Target.java +++ b/container-search/src/main/java/com/yahoo/search/federation/sourceref/Target.java @@ -23,9 +23,8 @@ public abstract class Target extends AbstractComponent { this(localId, false); } - public abstract SearchChainInvocationSpec responsibleSearchChain(Properties queryProperties) throws UnresolvedSearchChainException; + public abstract ResolveResult responsibleSearchChain(Properties queryProperties); public abstract String searchRefDescription(); abstract void freeze(); - } diff --git a/container-search/src/main/java/com/yahoo/search/federation/sourceref/UnresolvedProviderException.java b/container-search/src/main/java/com/yahoo/search/federation/sourceref/UnresolvedProviderException.java deleted file mode 100644 index aa21ad3b369..00000000000 --- a/container-search/src/main/java/com/yahoo/search/federation/sourceref/UnresolvedProviderException.java +++ /dev/null @@ -1,20 +0,0 @@ -// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.search.federation.sourceref; - -import com.yahoo.component.ComponentId; -import com.yahoo.component.ComponentSpecification; - -/** - * @author Tony Vaagenes - */ -@SuppressWarnings("serial") -class UnresolvedProviderException extends UnresolvedSearchChainException { - UnresolvedProviderException(String msg) { - super(msg); - } - - static UnresolvedSearchChainException createForMissingProvider(ComponentId source, - ComponentSpecification provider) { - return new UnresolvedProviderException("No provider '" + provider + "' for source '" + source + "'."); - } -} diff --git a/container-search/src/main/java/com/yahoo/search/federation/sourceref/UnresolvedSearchChainException.java b/container-search/src/main/java/com/yahoo/search/federation/sourceref/UnresolvedSearchChainException.java deleted file mode 100644 index 0c8562e6032..00000000000 --- a/container-search/src/main/java/com/yahoo/search/federation/sourceref/UnresolvedSearchChainException.java +++ /dev/null @@ -1,12 +0,0 @@ -// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.search.federation.sourceref; - -/** - * Thrown if a search chain can not be resolved from one or more ids. - * @author Tony Vaagenes - */ -public class UnresolvedSearchChainException extends Exception { - public UnresolvedSearchChainException(String msg) { - super(msg); - } -} diff --git a/container-search/src/main/java/com/yahoo/search/federation/sourceref/UnresolvedSourceRefException.java b/container-search/src/main/java/com/yahoo/search/federation/sourceref/UnresolvedSourceRefException.java deleted file mode 100644 index fa2c1da13f0..00000000000 --- a/container-search/src/main/java/com/yahoo/search/federation/sourceref/UnresolvedSourceRefException.java +++ /dev/null @@ -1,18 +0,0 @@ -// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.search.federation.sourceref; - -import com.yahoo.component.ComponentSpecification; - -/** - * @author Tony Vaagenes - */ -class UnresolvedSourceRefException extends UnresolvedSearchChainException { - UnresolvedSourceRefException(String msg) { - super(msg); - } - - - static UnresolvedSearchChainException createForMissingSourceRef(ComponentSpecification source) { - return new UnresolvedSourceRefException("Could not resolve source ref '" + source + "'."); - } -} diff --git a/container-search/src/test/java/com/yahoo/search/federation/sourceref/SearchChainResolverTestCase.java b/container-search/src/test/java/com/yahoo/search/federation/sourceref/SearchChainResolverTestCase.java index d9046075f38..e5bbb48e807 100644 --- a/container-search/src/test/java/com/yahoo/search/federation/sourceref/SearchChainResolverTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/federation/sourceref/SearchChainResolverTestCase.java @@ -13,8 +13,8 @@ import java.util.Iterator; import java.util.SortedSet; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.fail; /** * @author Tony Vaagenes @@ -59,37 +59,33 @@ public class SearchChainResolverTestCase { @Test void require_error_message_for_invalid_source() { - try { - resolve("no-such-source"); - fail("Expected exception."); - } catch (UnresolvedSearchChainException e) { - assertEquals("Could not resolve source ref 'no-such-source'.", e.getMessage()); - } + var result = resolve("no-such-source"); + assertEquals("Could not resolve source ref 'no-such-source'.", result.errorMsg()); } @Test - void lookup_search_chain() throws Exception { - SearchChainInvocationSpec res = resolve(searchChainId.getName()); + void lookup_search_chain() { + SearchChainInvocationSpec res = resolve(searchChainId.getName()).invocationSpec(); assertEquals(searchChainId, res.searchChainId); } //TODO: TVT: @Test() - public void lookup_provider() throws Exception { - SearchChainInvocationSpec res = resolve(providerId.getName()); + public void lookup_provider() { + SearchChainInvocationSpec res = resolve(providerId.getName()).invocationSpec(); assertEquals(providerId, res.provider); assertNull(res.source); assertEquals(providerId, res.searchChainId); } @Test - void lookup_source() throws Exception { - SearchChainInvocationSpec res = resolve(sourceId.getName()); + void lookup_source() { + SearchChainInvocationSpec res = resolve(sourceId.getName()).invocationSpec(); assertIsSourceInProvider(res); } @Test - void lookup_source_search_chain_directly() throws Exception { - SearchChainInvocationSpec res = resolve(sourceChainInProviderId.stringValue()); + void lookup_source_search_chain_directly() { + SearchChainInvocationSpec res = resolve(sourceChainInProviderId.stringValue()).invocationSpec(); assertIsSourceInProvider(res); } @@ -100,8 +96,8 @@ public class SearchChainResolverTestCase { } @Test - void lookup_source_for_provider2() throws Exception { - SearchChainInvocationSpec res = resolve(sourceId.getName(), provider2Id.getName()); + void lookup_source_for_provider2() { + SearchChainInvocationSpec res = resolve(sourceId.getName(), provider2Id.getName()).invocationSpec(); assertEquals(provider2Id, res.provider); assertEquals(sourceId, res.source); assertEquals(sourceChainInProvider2Id, res.searchChainId); @@ -126,22 +122,24 @@ public class SearchChainResolverTestCase { return new PropertyMap(); } - private SearchChainInvocationSpec resolve(String sourceSpecification) throws UnresolvedSearchChainException { + private ResolveResult resolve(String sourceSpecification) { return resolve(sourceSpecification, emptySourceToProviderMap()); } - private SearchChainInvocationSpec resolve(String sourceSpecification, String providerSpecification) - throws UnresolvedSearchChainException { + private ResolveResult resolve(String sourceSpecification, String providerSpecification) { Properties sourceToProviderMap = emptySourceToProviderMap(); sourceToProviderMap.set("source." + sourceSpecification + ".provider", providerSpecification); return resolve(sourceSpecification, sourceToProviderMap); } - private SearchChainInvocationSpec resolve(String sourceSpecification, Properties sourceToProviderMap) - throws UnresolvedSearchChainException { - SearchChainInvocationSpec res = searchChainResolver.resolve( + private ResolveResult resolve(String sourceSpecification, Properties sourceToProviderMap) { + ResolveResult res = searchChainResolver.resolve( ComponentSpecification.fromString(sourceSpecification), sourceToProviderMap); - assertEquals(federationOptions, res.federationOptions); + if (res.invocationSpec() != null) { + assertEquals(federationOptions, res.invocationSpec().federationOptions); + } else { + assertNotNull(res.errorMsg()); + } return res; } diff --git a/container-search/src/test/java/com/yahoo/search/federation/sourceref/SourceRefResolverTestCase.java b/container-search/src/test/java/com/yahoo/search/federation/sourceref/SourceRefResolverTestCase.java index b32135afc94..95262937c01 100644 --- a/container-search/src/test/java/com/yahoo/search/federation/sourceref/SourceRefResolverTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/federation/sourceref/SourceRefResolverTestCase.java @@ -3,21 +3,16 @@ package com.yahoo.search.federation.sourceref; import com.yahoo.component.ComponentId; import com.yahoo.component.ComponentSpecification; -import com.yahoo.prelude.IndexFacts; -import com.yahoo.prelude.IndexModel; import com.yahoo.search.searchchain.model.federation.FederationOptions; import org.junit.jupiter.api.Test; -import java.util.ArrayList; +import java.util.Collection; import java.util.List; import java.util.Map; -import java.util.Set; -import java.util.TreeMap; import static com.yahoo.search.federation.sourceref.SearchChainResolverTestCase.emptySourceToProviderMap; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.fail; /** * Test for SourceRefResolver. @@ -47,49 +42,38 @@ public class SourceRefResolverTestCase { @Test void lookup_search_chain() throws Exception { - Set searchChains = resolve(cluster1); + List searchChains = resolve(cluster1); assertEquals(1, searchChains.size()); assertTrue(searchChainIds(searchChains).contains(cluster1)); } @Test void lookup_search_chains_for_document1() throws Exception { - Set searchChains = resolve("document1"); + List searchChains = resolve("document1"); assertEquals(2, searchChains.size()); assertTrue(searchChainIds(searchChains).containsAll(List.of(cluster1, cluster2))); } @Test void error_when_document_gives_cluster_without_matching_search_chain() { - try { - resolve("document3"); - fail("Expected exception"); - } catch (UnresolvedSearchChainException e) { - assertEquals("Failed to resolve cluster search chain 'cluster3' " + - "when using source ref 'document3' as a document name.", - e.getMessage()); - } + List result = resolve("document3"); + + assertEquals("Failed to resolve cluster search chain 'cluster3' " + + "when using source ref 'document3' as a document name.", + result.get(0).errorMsg()); } @Test void error_when_no_document_or_search_chain() { - try { - resolve("document4"); - fail("Expected exception"); - } catch (UnresolvedSearchChainException e) { - assertEquals("Could not resolve source ref 'document4'.", e.getMessage()); - } + List results = resolve("document4"); + assertEquals("Could not resolve source ref 'document4'.", results.get(0).errorMsg()); } - private List searchChainIds(Set searchChains) { - List names = new ArrayList<>(); - for (SearchChainInvocationSpec searchChain : searchChains) { - names.add(searchChain.searchChainId.stringValue()); - } - return names; + private List searchChainIds(Collection searchChains) { + return searchChains.stream().map(r -> r.invocationSpec().searchChainId.stringValue()).toList(); } - private Set resolve(String documentName) throws UnresolvedSearchChainException { + private List resolve(String documentName) { return sourceRefResolver.resolve(ComponentSpecification.fromString(documentName), emptySourceToProviderMap()); } -- cgit v1.2.3