Remove strict contracts on Vespa 8

5 files changed, 20 insertions, 217 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 c0f4060be45..adf03340c6c 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
@@ -58,7 +58,6 @@ import java.util.logging.Logger;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.yahoo.collections.CollectionUtil.first;
-import static com.yahoo.search.federation.StrictContractsConfig.PropagateSourceProperties;
 
 /**
  * This searcher takes a set of sources, looks them up in config and fire off the correct searchchains.
@@ -81,34 +80,26 @@ public class FederationSearcher extends ForkingSearcher {
     private static final List<CompoundName> queryAndHits = ImmutableList.of(Query.OFFSET, Query.HITS);
 
     private final SearchChainResolver searchChainResolver;
-    private final PropagateSourceProperties.Enum propagateSourceProperties;
     private final SourceRefResolver sourceRefResolver;
     private final CopyOnWriteHashMap<CompoundKey, CompoundName> map = new CopyOnWriteHashMap<>();
 
-    private final boolean strictSearchchain;
     private final TargetSelector<?> targetSelector;
     private final Clock clock = Clock.systemUTC();
 
     @Inject
-    public FederationSearcher(FederationConfig config, StrictContractsConfig strict,
-                              ComponentRegistry<TargetSelector> targetSelectors) {
-        this(createResolver(config), strict.searchchains(), strict.propagateSourceProperties(),
-             resolveSelector(config.targetSelector(), targetSelectors));
+    public FederationSearcher(FederationConfig config, ComponentRegistry<TargetSelector> targetSelectors) {
+        this(createResolver(config), resolveSelector(config.targetSelector(), targetSelectors));
     }
 
     // for testing
     public FederationSearcher(ComponentId id, SearchChainResolver searchChainResolver) {
-        this(searchChainResolver, false, PropagateSourceProperties.EVERY, null);
+        this(searchChainResolver, null);
     }
 
     private FederationSearcher(SearchChainResolver searchChainResolver,
-                               boolean strictSearchchain,
-                               PropagateSourceProperties.Enum propagateSourceProperties,
                                TargetSelector targetSelector) {
         this.searchChainResolver = searchChainResolver;
         sourceRefResolver = new SourceRefResolver(searchChainResolver);
-        this.strictSearchchain = strictSearchchain;
-        this.propagateSourceProperties = propagateSourceProperties;
         this.targetSelector = targetSelector;
     }
 
@@ -221,13 +212,7 @@ public class FederationSearcher extends ForkingSearcher {
         if (timeout <= 0) return Optional.empty();
 
         Execution newExecution = new Execution(target.getChain(), execution.context());
-        Result result;
-        if (strictSearchchain) {
-            query.resetTimeout();
-            result = newExecution.search(createFederationQuery(query, query, Window.from(query), timeout, target));
-        } else {
-            result = newExecution.search(cloneFederationQuery(query, Window.from(query), timeout, target));
-        }
+        Result result = newExecution.search(cloneFederationQuery(query, Window.from(query), timeout, target));
         target.modifyTargetResult(result);
         return Optional.of(result);
     }
@@ -265,17 +250,7 @@ public class FederationSearcher extends ForkingSearcher {
 
         outgoing.setTimeout(timeout);
 
-        switch (propagateSourceProperties) {
-            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:
-                propagatePerSourceQueryProperties(query, outgoing, window, sourceName, providerName, queryAndHits);
-                break;
-        }
+        propagatePerSourceQueryProperties(query, outgoing, window, sourceName, providerName);
 
         //TODO: FederationTarget
         //TODO: only for target produced by this, not others
@@ -284,45 +259,13 @@ public class FederationSearcher extends ForkingSearcher {
     }
 
     private void propagatePerSourceQueryProperties(Query original, Query outgoing, Window window,
-                                                   String sourceName, String providerName,
-                                                   List<CompoundName> queryProperties) {
-        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);
-            }
-        }
-    }
-
-    private Object getSourceOrProviderProperty(Query query,
-                                               CompoundName propertyName,
-                                               String sourceName,
-                                               String providerName,
-                                               Object defaultValue) {
-        Object result = getProperty(query, new SourceKey(sourceName, propertyName.toString()));
-        if (result == null)
-            result = getProperty(query, new ProviderKey(providerName, propertyName.toString()));
-        if (result == null)
-            result = defaultValue;
-        return result;
-    }
-
-    private Object getProperty(Query query, CompoundKey key) {
-        CompoundName name = map.get(key);
-        if (name == null) {
-            name = new CompoundName(key.toString());
-            map.put(key, name);
-        }
-        return query.properties().get(name);
+                                                   String sourceName, String providerName) {
+        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));
     }
 
     private ErrorMessage missingSearchChainsErrorMessage(List<UnresolvedSearchChainException> unresolvedSearchChainExceptions) {
@@ -491,7 +434,7 @@ public class FederationSearcher extends ForkingSearcher {
     private HitOrderer dirtyCopyIfModifiedOrderer(HitGroup group, HitOrderer orderer) {
         if (orderer != null) {
             HitOrderer old = group.getOrderer();
-            if ((old == null) || ! orderer.equals(old)) {
+            if (! orderer.equals(old)) {
                 group.setOrderer(orderer);
             }
         }
@@ -792,55 +735,6 @@ public class FederationSearcher extends ForkingSearcher {
         }
     }
 
-    private static class SourceKey extends CompoundKey {
-
-        public static final String SOURCE = "source.";
-
-        SourceKey(String sourceName, String propertyName) {
-            super(sourceName, propertyName);
-        }
-
-        @Override
-        public int hashCode() {
-            return super.hashCode() ^ 7;
-        }
-
-        @Override
-        public boolean equals(Object o) {
-            return (o instanceof SourceKey) && super.equals(o);
-        }
-
-        @Override
-        public String toString() {
-            return SOURCE + super.toString();
-        }
-    }
-
-    private static class ProviderKey extends CompoundKey {
-
-        public static final String PROVIDER = "provider.";
-
-        ProviderKey(String sourceName, String propertyName) {
-            super(sourceName, propertyName);
-        }
-
-        @Override
-        public int hashCode() {
-            return super.hashCode() ^ 17;
-        }
-
-        @Override
-        public boolean equals(Object o) {
-            return (o instanceof ProviderKey) && super.equals(o);
-        }
-
-        @Override
-        public String toString() {
-            return PROVIDER + super.toString();
-        }
-
-    }
-    
     private static class Window {
         
         private final int hits;
diff --git a/container-search/src/main/resources/configdefinitions/search.federation.strict-contracts.def b/container-search/src/main/resources/configdefinitions/search.federation.strict-contracts.def
deleted file mode 100644
index 9d0e70f208a..00000000000
--- a/container-search/src/main/resources/configdefinitions/search.federation.strict-contracts.def
+++ /dev/null
@@ -1,20 +0,0 @@
-# Copyright Yahoo. 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
-## signals the basic contract is sufficient, and allows the container to
-## activate optimizations based on this knowledge.
-
-## Strict contracts for search chains, do not clone the query if it at all
-## can be construed to be unnecessary.
-searchchains bool default=false
-
-# 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 7dad3c9b52b..4fffc450e74 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
@@ -12,7 +12,6 @@ import com.yahoo.component.chain.Chain;
 import com.yahoo.component.provider.ComponentRegistry;
 import com.yahoo.search.federation.FederationConfig;
 import com.yahoo.container.QrSearchersConfig;
-import com.yahoo.search.federation.StrictContractsConfig;
 import com.yahoo.search.Query;
 import com.yahoo.search.Result;
 import com.yahoo.prelude.fastsearch.FastHit;
@@ -113,10 +112,8 @@ public class BlendingSearcherTestCase {
                                 entry.getValue()));
             }
 
-            StrictContractsConfig contracts = new StrictContractsConfig.Builder().build();
-
             FederationSearcher fedSearcher =
-                    new FederationSearcher(new FederationConfig(builder), contracts, new ComponentRegistry<>());
+                    new FederationSearcher(new FederationConfig(builder), new ComponentRegistry<>());
             BlendingSearcher blendingSearcher = new BlendingSearcher(blendingField);
             blendingChain = new SearchChain(ComponentId.createAnonymousComponentId("blendingChain"), blendingSearcher, fedSearcher);
             return true;
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 ef9e0311e59..4bb6db40c66 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
@@ -20,8 +20,6 @@ import com.yahoo.search.searchchain.Execution;
 import com.yahoo.search.searchchain.Execution.Context;
 import com.yahoo.search.searchchain.model.federation.FederationOptions;
 
-import org.junit.After;
-import org.junit.Before;
 import org.junit.Test;
 
 import java.util.Arrays;
@@ -173,7 +171,6 @@ public class FederationSearcherTest {
 
         FederationSearcher searcher = new FederationSearcher(
                 new FederationConfig(new FederationConfig.Builder().targetSelector(targetSelectorId.toString())),
-                new StrictContractsConfig(new StrictContractsConfig.Builder()),
                 targetSelectors);
 
         Query query = new Query();
@@ -192,7 +189,6 @@ public class FederationSearcherTest {
 
         FederationSearcher searcher = new FederationSearcher(
                 new FederationConfig(new FederationConfig.Builder().targetSelector(targetSelectorId.toString())),
-                new StrictContractsConfig(new StrictContractsConfig.Builder()),
                 targetSelectors);
 
         Query query = new Query();
diff --git a/container-search/src/test/java/com/yahoo/search/federation/FederationSearcherTestCase.java b/container-search/src/test/java/com/yahoo/search/federation/FederationSearcherTestCase.java
index 8c7e5fd59c6..c2feb16df04 100644
--- a/container-search/src/test/java/com/yahoo/search/federation/FederationSearcherTestCase.java
+++ b/container-search/src/test/java/com/yahoo/search/federation/FederationSearcherTestCase.java
@@ -11,7 +11,6 @@ import com.yahoo.search.federation.sourceref.SearchChainResolver;
 import com.yahoo.search.query.profile.QueryProfile;
 import com.yahoo.search.result.ErrorMessage;
 import com.yahoo.search.result.Hit;
-import com.yahoo.search.result.HitGroup;
 import com.yahoo.search.searchchain.Execution;
 import com.yahoo.search.searchchain.SearchChain;
 import com.yahoo.search.searchchain.SearchChainRegistry;
@@ -26,12 +25,10 @@ import org.junit.Test;
 
 import java.util.List;
 
-import static com.yahoo.search.federation.StrictContractsConfig.PropagateSourceProperties;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 
 /**
@@ -88,21 +85,7 @@ public class FederationSearcherTestCase {
     }
 
     private Searcher createFederationSearcher() {
-        return buildFederation(new StrictContractsConfig(new StrictContractsConfig.Builder()));
-    }
-
-    private Searcher createFederationSearcher(PropagateSourceProperties.Enum propagateSourceProperties) {
-        return buildFederation(new StrictContractsConfig(new StrictContractsConfig.Builder().propagateSourceProperties(propagateSourceProperties)));
-    }
-
-    private Searcher createStrictFederationSearcher() {
-        StrictContractsConfig.Builder builder = new StrictContractsConfig.Builder();
-        builder.searchchains(true);
-        return buildFederation(new StrictContractsConfig(builder));
-    }
-
-    private Searcher buildFederation(StrictContractsConfig contracts) throws RuntimeException {
-        return new FederationSearcher(new FederationConfig(builder), contracts, new ComponentRegistry<>());
+        return new FederationSearcher(new FederationConfig(builder), new ComponentRegistry<>());
     }
 
     private SearchChain createSearchChain(ComponentId chainId,Searcher searcher) {
@@ -161,10 +144,7 @@ public class FederationSearcherTestCase {
         }, SOURCE2);
 
         return new Chain<>("default",
-                new FederationSearcher(new FederationConfig(builder),
-                        new StrictContractsConfig(
-                                new StrictContractsConfig.Builder().searchchains(strictContracts)),
-                        new ComponentRegistry<>()));
+                           new FederationSearcher(new FederationConfig(builder), new ComponentRegistry<>()));
     }
 
     @Test
@@ -199,19 +179,8 @@ public class FederationSearcherTestCase {
     }
 
     @Test
-    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());
-        assertNull(result.hits().get(0).getQuery().properties().get("custom"));
-    }
-
-    @Test
-    public void testPropertyPropagation_every() {
-        Result result = searchWithPropertyPropagation(PropagateSourceProperties.EVERY);
+    public void testPropertyPropagation() {
+        Result result = searchWithPropertyPropagation();
 
         assertEquals("source:mySource1", result.hits().get(0).getId().stringValue());
         assertEquals("source:mySource2", result.hits().get(1).getId().stringValue());
@@ -228,10 +197,10 @@ public class FederationSearcherTestCase {
         assertNull(result.hits().get(1).getQuery().getPresentation().getSummary());
     }
 
-    private Result searchWithPropertyPropagation(PropagateSourceProperties.Enum propagateSourceProperties) {
+    private Result searchWithPropertyPropagation() {
         addChained(new MockSearcher(), "mySource1");
         addChained(new MockSearcher(), "mySource2");
-        Chain<Searcher> mainChain = new Chain<>("default", createFederationSearcher(propagateSourceProperties));
+        Chain<Searcher> mainChain = new Chain<>("default", createFederationSearcher());
 
         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"));
 
@@ -241,39 +210,6 @@ public class FederationSearcherTestCase {
     }
 
     @Test
-    public void testDisablePropertyPropagation() {
-        Result result = searchWithPropertyPropagation(PropagateSourceProperties.NONE);
-
-        assertNull(result.hits().get(0).getQuery().getPresentation().getSummary());
-    }
-    
-    @Test
-    public void testNoCloning() {
-        String sourceName = "cloningcheck";
-        Query query = new Query(QueryTestCase.httpEncode("?query=test&sources=" + sourceName));
-        addChained(new QueryCheckSearcher(query), sourceName);
-        addChained(new MockSearcher(), "mySource1");
-        Chain<Searcher> mainChain = new Chain<>("default", createStrictFederationSearcher());
-        Result result = new Execution(mainChain, Execution.Context.createContextStub(chainRegistry)).search(query);
-        HitGroup h = (HitGroup) result.hits().get(0);
-        assertNull(h.getErrorHit());
-        assertSame(QueryCheckSearcher.OK, h.get(0).getField(QueryCheckSearcher.STATUS));
-
-        mainChain = new Chain<>("default", createFederationSearcher());
-        result = new Execution(mainChain, Execution.Context.createContextStub(chainRegistry)).search(query);
-        h = (HitGroup) result.hits().get(0);
-        assertSame(QueryCheckSearcher.FEDERATION_SEARCHER_HAS_CLONED_THE_QUERY, h.getError().getDetailedMessage());
-
-        query = new Query(QueryTestCase.httpEncode("?query=test&sources=" + sourceName + ",mySource1"));
-        addChained(new QueryCheckSearcher(query), sourceName);
-        result = new Execution(mainChain, Execution.Context.createContextStub(chainRegistry)).search(query);
-        h = (HitGroup) result.hits().get(0);
-        assertEquals("source:" + sourceName, h.getId().stringValue());
-        assertSame(QueryCheckSearcher.FEDERATION_SEARCHER_HAS_CLONED_THE_QUERY, h.getError().getDetailedMessage());
-        assertEquals("source:mySource1", result.hits().get(1).getId().stringValue());
-    }
-
-    @Test
     public void testTopLevelHitGroupFieldPropagation() {
         addChained(new MockSearcher(), "mySource1");
         addChained(new AnotherMockSearcher(), "mySource2");