Merge pull request #11932 from vespa-engine/bratseth/propagate-all-properties

Propagate all source and provider properties by default

4 files changed, 52 insertions, 20 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 c3ede4fe20a..d0b3189a79c 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
@@ -110,7 +110,7 @@ public class FederationSearcher extends ForkingSearcher {
 
     // for testing
     public FederationSearcher(ComponentId id, SearchChainResolver searchChainResolver) {
-        this(searchChainResolver, false, PropagateSourceProperties.ALL, null);
+        this(searchChainResolver, false, PropagateSourceProperties.EVERY, null);
     }
 
     private FederationSearcher(SearchChainResolver searchChainResolver,
@@ -271,7 +271,10 @@ public class FederationSearcher extends ForkingSearcher {
         outgoing.setTimeout(timeout);
 
         switch (propagateSourceProperties) {
-            case ALL:
+            case EVERY:
+                propagatePerSourceQueryProperties(query, outgoing, window, sourceName, providerName, null);
+                break;
+            case NATIVE: case ALL:
                 propagatePerSourceQueryProperties(query, outgoing, window, sourceName, providerName, Query.nativeProperties);
                 break;
             case OFFSET_HITS:
@@ -288,10 +291,21 @@ public class FederationSearcher extends ForkingSearcher {
     private void propagatePerSourceQueryProperties(Query original, Query outgoing, Window window,
                                                    String sourceName, String providerName,
                                                    List<CompoundName> queryProperties) {
-        for (CompoundName key : queryProperties) {
-            Object value = getSourceOrProviderProperty(original, key, sourceName, providerName, window.get(key));
-            if (value != null)
-                outgoing.properties().set(key, value);
+        if (queryProperties == null) {
+            outgoing.setHits(window.hits);
+            outgoing.setOffset(window.offset);
+            original.properties().listProperties(CompoundName.fromComponents("provider", providerName)).forEach((k, v) ->
+                outgoing.properties().set(k, v));
+            original.properties().listProperties(CompoundName.fromComponents("source", sourceName)).forEach((k, v) ->
+                outgoing.properties().set(k, v));
+        }
+        else {
+            for (CompoundName key : queryProperties) {
+                Object value = getSourceOrProviderProperty(original, key, sourceName, providerName, window.get(key));
+                if (value != null)
+                    outgoing.properties().set(key, value);
+                if (value != null) System.out.println("Setting " + key + " = " + value);
+            }
         }
     }
 
@@ -319,7 +333,7 @@ public class FederationSearcher extends ForkingSearcher {
 
     private ErrorMessage missingSearchChainsErrorMessage(List<UnresolvedSearchChainException> unresolvedSearchChainExceptions) {
         String message = String.join(" ", getMessagesSet(unresolvedSearchChainExceptions)) +
-                      " Valid source refs are " + String.join(", ", allSourceRefDescriptions()) +'.';
+                                     " Valid source refs are " + String.join(", ", allSourceRefDescriptions()) +'.';
         return ErrorMessage.createInvalidQueryParameter(message);
     }
 
diff --git a/container-search/src/main/resources/configdefinitions/strict-contracts.def b/container-search/src/main/resources/configdefinitions/strict-contracts.def
index f9dd788c054..5ceb37db8d1 100644
--- a/container-search/src/main/resources/configdefinitions/strict-contracts.def
+++ b/container-search/src/main/resources/configdefinitions/strict-contracts.def
@@ -1,6 +1,7 @@
 # Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 namespace=search.federation
 
+## DEPRECATED: This config will be removed on Vespa 8
 ## A config to control whether to activate strict adherence to public contracts
 ## in the container. Usually, the container tries to do a best effort of hiding
 ## some undesirable effects of the the public contracts. Modifying this config
@@ -11,11 +12,9 @@ namespace=search.federation
 ## can be construed to be unnecessary.
 searchchains bool default=false
 
-# WARNING: Beta feature, might be removed soon.
-# Propagate source.(sourceName).{QueryProperties.PER_SOURCE_QUERY_PROPERTIES} and
-#           provider.(providerName).{QueryProperties.PER_SOURCE_QUERY_PROPERTIES}
-# to the outgoing query.
-# All means all in QueryProperties.PER_SOURCE_QUERY_PROPERTIES
-# OFFSET_HITS means  {Query.HITS, Query.OFFSET}
-# NONE means {}
-propagateSourceProperties enum {ALL, OFFSET_HITS, NONE} default=ALL
+# EVERY, // Propagate any property starting by source.[sourceName] and provider.[providerName]
+# NATIVE, // Propagate native properties only
+# ALL, // Deprecated synonym of NATIVE
+# OFFSET_HITS, // Propagate offset ands hits only
+# NONE // propagate no properties
+propagateSourceProperties enum {EVERY, NATIVE, ALL, OFFSET_HITS, NONE} default=EVERY
diff --git a/container-search/src/test/java/com/yahoo/prelude/searcher/test/BlendingSearcherTestCase.java b/container-search/src/test/java/com/yahoo/prelude/searcher/test/BlendingSearcherTestCase.java
index 9da1c184505..0086f1b3571 100644
--- a/container-search/src/test/java/com/yahoo/prelude/searcher/test/BlendingSearcherTestCase.java
+++ b/container-search/src/test/java/com/yahoo/prelude/searcher/test/BlendingSearcherTestCase.java
@@ -108,7 +108,7 @@ public class BlendingSearcherTestCase {
                                 entry.getValue()));
             }
 
-            StrictContractsConfig contracts = new StrictContractsConfig(new StrictContractsConfig.Builder());
+            StrictContractsConfig contracts = new StrictContractsConfig.Builder().build();
 
             FederationSearcher fedSearcher =
                     new FederationSearcher(new FederationConfig(builder), contracts, new ComponentRegistry<>());
@@ -124,7 +124,6 @@ public class BlendingSearcherTestCase {
 
     @Test
     public void testitTwoPhase() {
-
         DocumentSourceSearcher chain1 = new DocumentSourceSearcher();
         DocumentSourceSearcher chain2 = new DocumentSourceSearcher();
         DocumentSourceSearcher chain3 = new DocumentSourceSearcher();
diff --git a/container-search/src/test/java/com/yahoo/search/federation/test/FederationSearcherTestCase.java b/container-search/src/test/java/com/yahoo/search/federation/test/FederationSearcherTestCase.java
index 111b7a8eb69..65cb4dff1f8 100644
--- a/container-search/src/test/java/com/yahoo/search/federation/test/FederationSearcherTestCase.java
+++ b/container-search/src/test/java/com/yahoo/search/federation/test/FederationSearcherTestCase.java
@@ -201,13 +201,33 @@ public class FederationSearcherTestCase {
     }
 
     @Test
-    public void testPropertyPropagation() {
-        Result result = searchWithPropertyPropagation(PropagateSourceProperties.ALL);
+    public void testPropertyPropagation_native() {
+        Result result = searchWithPropertyPropagation(PropagateSourceProperties.NATIVE);
 
         assertEquals("source:mySource1", result.hits().get(0).getId().stringValue());
         assertEquals("source:mySource2", result.hits().get(1).getId().stringValue());
         assertEquals("nalle", result.hits().get(0).getQuery().getPresentation().getSummary());
         assertNull(result.hits().get(1).getQuery().getPresentation().getSummary());
+        assertEquals(null, result.hits().get(0).getQuery().properties().get("custom"));
+    }
+
+    @Test
+    public void testPropertyPropagation_every() {
+        Result result = searchWithPropertyPropagation(PropagateSourceProperties.EVERY);
+
+        assertEquals("source:mySource1", result.hits().get(0).getId().stringValue());
+        assertEquals("source:mySource2", result.hits().get(1).getId().stringValue());
+        assertEquals("nalle", result.hits().get(0).getQuery().getPresentation().getSummary());
+        assertEquals("foo", result.hits().get(0).getQuery().properties().get("customSourceProperty"));
+        assertEquals(null,  result.hits().get(1).getQuery().properties().get("customSourceProperty"));
+        assertEquals(null,  result.hits().get(0).getQuery().properties().get("custom.source.property"));
+        assertEquals("bar", result.hits().get(1).getQuery().properties().get("custom.source.property"));
+        assertEquals(13, result.hits().get(0).getQuery().properties().get("hits"));
+        assertEquals(1, result.hits().get(0).getQuery().properties().get("offset"));
+        assertEquals(10, result.hits().get(1).getQuery().properties().get("hits"));
+        assertEquals(0, result.hits().get(1).getQuery().properties().get("offset"));
+
+        assertNull(result.hits().get(1).getQuery().getPresentation().getSummary());
     }
 
     private Result searchWithPropertyPropagation(PropagateSourceProperties.Enum propagateSourceProperties) {
@@ -215,7 +235,7 @@ public class FederationSearcherTestCase {
         addChained(new MockSearcher(), "mySource2");
         Chain<Searcher> mainChain = new Chain<>("default", createFederationSearcher(propagateSourceProperties));
 
-        Query q = new Query(QueryTestCase.httpEncode("?query=test&source.mySource1.presentation.summary=nalle"));
+        Query q = new Query(QueryTestCase.httpEncode("?query=test&source.mySource1.presentation.summary=nalle&source.mySource1.customSourceProperty=foo&source.mySource2.custom.source.property=bar&source.mySource1.hits=13&source.mySource1.offset=1"));
 
         Result result = new Execution(mainChain, Execution.Context.createContextStub(chainRegistry, null)).search(q);
         assertNull(result.hits().getError());