From e246e4143d427b6c003b6a028826ee02bccbdb11 Mon Sep 17 00:00:00 2001 From: pgomulka Date: Tue, 27 Jul 2021 09:19:06 +0200 Subject: [PATCH 1/8] [Rest Api Compatibility] Make query registration easier Refactoring to NamedXContentRegistry to make it easier to register new query builders. It removes the concept of separate compatibel namedXContentRegistry and adds a second dimension - restApiVersion - to registry in NamedXContentRegistry. This makes the design similar to the solution in ObjectParser where the field parser lookup map also needs has a restApiVersion relates #51816 --- .../xcontent/NamedXContentRegistry.java | 106 ++++++++---------- .../java/org/elasticsearch/node/Node.java | 10 +- .../elasticsearch/search/SearchModule.java | 2 +- .../CompatibleNamedXContentRegistryTests.java | 23 ++-- .../org/elasticsearch/node/NodeTests.java | 22 ++-- .../search/SearchModuleTests.java | 74 ++++++++++++ 6 files changed, 155 insertions(+), 82 deletions(-) diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/NamedXContentRegistry.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/NamedXContentRegistry.java index 5f5f59a21db24..c12bf8cb4a98e 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/NamedXContentRegistry.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/NamedXContentRegistry.java @@ -12,12 +12,11 @@ import org.elasticsearch.core.RestApiVersion; import java.io.IOException; -import java.util.ArrayList; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.function.Function; import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; @@ -43,92 +42,84 @@ public static class Entry { /** A name for the entry which is unique within the {@link #categoryClass}. */ public final ParseField name; + public final Function restApiCompatibility; + /** A parser capability of parser the entry's class. */ private final ContextParser parser; - /** Creates a new entry which can be stored by the registry. */ + /** + * Creates a new entry which can be stored by the registry. + */ public Entry(Class categoryClass, ParseField name, CheckedFunction parser) { - this.categoryClass = Objects.requireNonNull(categoryClass); - this.name = Objects.requireNonNull(name); - this.parser = Objects.requireNonNull((p, c) -> parser.apply(p)); + this(categoryClass, name, (p, c) -> parser.apply(p), + RestApiVersion.onOrAfter(RestApiVersion.minimumSupported())); + } + + public Entry(Class categoryClass, ParseField name, CheckedFunction parser, + Function restApiCompatibility) { + this(categoryClass, name, (p, c) -> parser.apply(p), restApiCompatibility); } /** * Creates a new entry which can be stored by the registry. * Prefer {@link Entry#Entry(Class, ParseField, CheckedFunction)} unless you need a context to carry around while parsing. */ public Entry(Class categoryClass, ParseField name, ContextParser parser) { + this(categoryClass, name, parser, + RestApiVersion.onOrAfter(RestApiVersion.minimumSupported())); + } + + public Entry(Class categoryClass, ParseField name, ContextParser parser, + Function restApiCompatibility) { this.categoryClass = Objects.requireNonNull(categoryClass); this.name = Objects.requireNonNull(name); this.parser = Objects.requireNonNull(parser); + this.restApiCompatibility = restApiCompatibility; } } - private final Map, Map> registry; - private final Map, Map> compatibleRegistry; + private final Map, Map>> registry; - public NamedXContentRegistry(List entries){ - this(entries, Collections.emptyList()); - } - public NamedXContentRegistry(List entries, List compatibleEntries) { - this.registry = unmodifiableMap(getRegistry(entries)); - this.compatibleRegistry = unmodifiableMap(getCompatibleRegistry(compatibleEntries)); + public NamedXContentRegistry(List entries) { + this.registry = unmodifiableMap(createRegistry(entries)); } - private Map, Map> getCompatibleRegistry(List compatibleEntries) { - Map, Map> compatibleRegistry = new HashMap<>(registry); - List unseenEntries = new ArrayList<>(); - compatibleEntries.forEach(entry -> { - Map parsers = compatibleRegistry.get(entry.categoryClass); - if (parsers == null) { - unseenEntries.add(entry); - } else { - Map parsersCopy = new HashMap<>(parsers); - for (String name : entry.name.getAllNamesIncludedDeprecated()) { - parsersCopy.put(name, entry); //override the parser for the given name - } - compatibleRegistry.put(entry.categoryClass, parsersCopy); - } - } - ); - compatibleRegistry.putAll(getRegistry(unseenEntries)); - return compatibleRegistry; - } - private Map, Map> getRegistry(List entries){ + private Map, Map>> createRegistry(List entries){ if (entries.isEmpty()) { return emptyMap(); } - entries = new ArrayList<>(entries); - entries.sort((e1, e2) -> e1.categoryClass.getName().compareTo(e2.categoryClass.getName())); - Map, Map> registry = new HashMap<>(); - Map parsers = null; - Class currentCategory = null; + Map, Map>> registry = new HashMap<>(); for (Entry entry : entries) { - if (currentCategory != entry.categoryClass) { - if (currentCategory != null) { - // we've seen the last of this category, put it into the big map - registry.put(currentCategory, unmodifiableMap(parsers)); - } - parsers = new HashMap<>(); - currentCategory = entry.categoryClass; - } - for (String name : entry.name.getAllNamesIncludedDeprecated()) { - Object old = parsers.put(name, entry); - if (old != null) { - throw new IllegalArgumentException("NamedXContent [" + currentCategory.getName() + "][" + entry.name + "]" + - " is already registered for [" + old.getClass().getName() + "]," + - " cannot register [" + entry.parser.getClass().getName() + "]"); + if (RestApiVersion.minimumSupported().matches(entry.restApiCompatibility)) { + registerParsers(registry, entry, name, RestApiVersion.minimumSupported()); + } + if (RestApiVersion.current().matches(entry.restApiCompatibility)) { + registerParsers(registry, entry, name, RestApiVersion.current()); } } } - // handle the last category - registry.put(currentCategory, unmodifiableMap(parsers)); return registry; } + private void registerParsers(Map, Map>> registry, + Entry entry, + String name, + RestApiVersion restApiVersion) { + final Map, Map> classRegistry = + registry.computeIfAbsent(restApiVersion, (v) -> new HashMap<>()); + final Map parsers = + classRegistry.computeIfAbsent(entry.categoryClass, (v) -> new HashMap<>()); + Object old = parsers.put(name, entry); + if (old != null) { + throw new IllegalArgumentException("NamedXContent [" + entry.categoryClass.getName() + "][" + entry.name + "]" + + " is already registered for [" + old.getClass().getName() + "]," + + " cannot register [" + entry.parser.getClass().getName() + "]"); + } + } + /** * Parse a named object, throwing an exception if the parser isn't found. Throws an {@link NamedObjectNotFoundException} if the * {@code categoryClass} isn't registered because this is almost always a bug. Throws an {@link NamedObjectNotFoundException} if the @@ -137,9 +128,8 @@ private Map, Map> getRegistry(List entries){ * @throws NamedObjectNotFoundException if the categoryClass or name is not registered */ public T parseNamedObject(Class categoryClass, String name, XContentParser parser, C context) throws IOException { - - Map parsers = parser.getRestApiVersion() == RestApiVersion.minimumSupported() ? - compatibleRegistry.get(categoryClass) : registry.get(categoryClass); + Map parsers = registry.getOrDefault(parser.getRestApiVersion(), emptyMap()) + .get(categoryClass); if (parsers == null) { if (registry.isEmpty()) { // The "empty" registry will never work so we throw a better exception as a hint. diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index 66cdea8bdd9b5..a19b5920cc622 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -416,9 +416,9 @@ protected Node(final Environment initialEnvironment, searchModule.getNamedXContents().stream(), pluginsService.filterPlugins(Plugin.class).stream() .flatMap(p -> p.getNamedXContent().stream()), - ClusterModule.getNamedXWriteables().stream()) - .flatMap(Function.identity()).collect(toList()), - getCompatibleNamedXContents() + ClusterModule.getNamedXWriteables().stream(), + getCompatibleNamedXContents()) + .flatMap(Function.identity()).collect(toList()) ); final Map featuresMap = pluginsService .filterPlugins(SystemIndexPlugin.class) @@ -738,9 +738,9 @@ protected Node(final Environment initialEnvironment, } // package scope for testing - List getCompatibleNamedXContents() { + Stream getCompatibleNamedXContents() { return pluginsService.filterPlugins(Plugin.class).stream() - .flatMap(p -> p.getNamedXContentForCompatibility().stream()).collect(toList()); + .flatMap(p -> p.getNamedXContentForCompatibility().stream()); } protected TransportService newTransportService(Settings settings, Transport transport, ThreadPool threadPool, diff --git a/server/src/main/java/org/elasticsearch/search/SearchModule.java b/server/src/main/java/org/elasticsearch/search/SearchModule.java index 4f35bf10c94bb..e6d98d64d5519 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchModule.java +++ b/server/src/main/java/org/elasticsearch/search/SearchModule.java @@ -881,7 +881,7 @@ public static List getIntervalsSourceProviderNamed private void registerQuery(QuerySpec spec) { namedWriteables.add(new NamedWriteableRegistry.Entry(QueryBuilder.class, spec.getName().getPreferredName(), spec.getReader())); namedXContents.add(new NamedXContentRegistry.Entry(QueryBuilder.class, spec.getName(), - (p, c) -> spec.getParser().fromXContent(p))); + (p, c) -> spec.getParser().fromXContent(p), spec.getName().getForRestApiVersion())); } private void registerBoolQuery(ParseField name, Writeable.Reader reader) { diff --git a/server/src/test/java/org/elasticsearch/common/xcontent/CompatibleNamedXContentRegistryTests.java b/server/src/test/java/org/elasticsearch/common/xcontent/CompatibleNamedXContentRegistryTests.java index 72d6cf0797552..207e0edec22b8 100644 --- a/server/src/test/java/org/elasticsearch/common/xcontent/CompatibleNamedXContentRegistryTests.java +++ b/server/src/test/java/org/elasticsearch/common/xcontent/CompatibleNamedXContentRegistryTests.java @@ -16,7 +16,6 @@ import org.elasticsearch.test.rest.FakeRestRequest; import java.io.IOException; -import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Map; @@ -57,6 +56,8 @@ static class SubObject { private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( "parser1", false, a -> new SubObject((String) a[0])); + static final ParseField NAME = new ParseField("namedObjectName1") + .forRestApiVersion(RestApiVersion.onOrAfter(RestApiVersion.current())); static { PARSER.declareString(ConstructingObjectParser.constructorArg(), new ParseField("new_field")); @@ -77,7 +78,8 @@ static class OldSubObject extends SubObject { private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( "parser2", false, a -> new SubObject((String) a[0])); - + static final ParseField NAME = new ParseField("namedObjectName1") + .forRestApiVersion(RestApiVersion.equalTo(RestApiVersion.minimumSupported())); static { PARSER.declareString(ConstructingObjectParser.constructorArg(), new ParseField("old_field")); } @@ -93,8 +95,10 @@ public static SubObject parse(XContentParser parser) { public void testNotCompatibleRequest() throws IOException { NamedXContentRegistry registry = new NamedXContentRegistry( - Arrays.asList(new NamedXContentRegistry.Entry(SubObject.class, new ParseField("namedObjectName1"), SubObject::parse)), - Arrays.asList(new NamedXContentRegistry.Entry(SubObject.class, new ParseField("namedObjectName1"), OldSubObject::parse))); + List.of(new NamedXContentRegistry.Entry(SubObject.class, SubObject.NAME, SubObject::parse, + SubObject.NAME.getForRestApiVersion()), + new NamedXContentRegistry.Entry(SubObject.class, OldSubObject.NAME, OldSubObject::parse, + OldSubObject.NAME.getForRestApiVersion()))); XContentBuilder b = XContentBuilder.builder(XContentType.JSON.xContent()); b.startObject(); @@ -120,13 +124,14 @@ public void testNotCompatibleRequest() throws IOException { ParentObject parse = ParentObject.parse(p); assertThat(parse.subObject.field, equalTo("value1")); } - } public void testCompatibleRequest() throws IOException { - NamedXContentRegistry compatibleRegistry = new NamedXContentRegistry( - Arrays.asList(new NamedXContentRegistry.Entry(SubObject.class, new ParseField("namedObjectName1"), SubObject::parse)), - Arrays.asList(new NamedXContentRegistry.Entry(SubObject.class, new ParseField("namedObjectName1"), OldSubObject::parse))); + NamedXContentRegistry registry = new NamedXContentRegistry( + List.of(new NamedXContentRegistry.Entry(SubObject.class, SubObject.NAME, SubObject::parse, + SubObject.NAME.getForRestApiVersion()), + new NamedXContentRegistry.Entry(SubObject.class, OldSubObject.NAME, OldSubObject::parse, + OldSubObject.NAME.getForRestApiVersion()))); XContentBuilder b = XContentBuilder.builder(XContentType.JSON.xContent()); b.startObject(); @@ -141,7 +146,7 @@ public void testCompatibleRequest() throws IOException { String.valueOf(RestApiVersion.minimumSupported().major))); List mediaTypeList = Collections.singletonList(mediaType); - RestRequest restRequest2 = new FakeRestRequest.Builder(compatibleRegistry) + RestRequest restRequest2 = new FakeRestRequest.Builder(registry) .withContent(BytesReference.bytes(b), RestRequest.parseContentType(mediaTypeList)) .withPath("/foo") .withHeaders(Map.of("Content-Type", mediaTypeList, "Accept", mediaTypeList)) diff --git a/server/src/test/java/org/elasticsearch/node/NodeTests.java b/server/src/test/java/org/elasticsearch/node/NodeTests.java index b76b95280cdb6..b97b8dc33fe40 100644 --- a/server/src/test/java/org/elasticsearch/node/NodeTests.java +++ b/server/src/test/java/org/elasticsearch/node/NodeTests.java @@ -12,14 +12,14 @@ import org.elasticsearch.bootstrap.BootstrapCheck; import org.elasticsearch.bootstrap.BootstrapContext; import org.elasticsearch.cluster.ClusterName; -import org.elasticsearch.common.xcontent.ParseField; import org.elasticsearch.common.breaker.CircuitBreaker; -import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.common.network.NetworkModule; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.transport.BoundTransportAddress; import org.elasticsearch.common.xcontent.ContextParser; import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.common.xcontent.ParseField; +import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.env.Environment; import org.elasticsearch.index.IndexService; import org.elasticsearch.index.engine.Engine.Searcher; @@ -44,6 +44,7 @@ import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.stream.Collectors; import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS; import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS; @@ -55,6 +56,7 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; +import static org.mockito.Mockito.mock; @LuceneTestCase.SuppressFileSystems(value = "ExtrasFS") public class NodeTests extends ESTestCase { @@ -340,25 +342,25 @@ interface MockRestApiVersion { RestApiVersion minimumRestCompatibilityVersion(); } - static MockRestApiVersion MockCompatibleVersion = Mockito.mock(MockRestApiVersion.class); + static MockRestApiVersion MockCompatibleVersion = mock(MockRestApiVersion.class); static NamedXContentRegistry.Entry v7CompatibleEntries = new NamedXContentRegistry.Entry(Integer.class, - new ParseField("name"), Mockito.mock(ContextParser.class)); + new ParseField("name"), mock(ContextParser.class)); static NamedXContentRegistry.Entry v8CompatibleEntries = new NamedXContentRegistry.Entry(Integer.class, - new ParseField("name2"), Mockito.mock(ContextParser.class)); + new ParseField("name2"), mock(ContextParser.class)); public static class TestRestCompatibility1 extends Plugin { @Override public List getNamedXContentForCompatibility() { // real plugin will use CompatibleVersion.minimumRestCompatibilityVersion() - if (/*CompatibleVersion.minimumRestCompatibilityVersion()*/ + if (/*RestApiVersion.minimumSupported() == */ MockCompatibleVersion.minimumRestCompatibilityVersion().equals(RestApiVersion.V_7)) { //return set of N-1 entries return List.of(v7CompatibleEntries); } // after major release, new compatible apis can be added before the old ones are removed. - if (/*CompatibleVersion.minimumRestCompatibilityVersion()*/ + if (/*RestApiVersion.minimumSupported() == */ MockCompatibleVersion.minimumRestCompatibilityVersion().equals(RestApiVersion.V_8)) { return List.of(v8CompatibleEntries); @@ -381,7 +383,8 @@ public void testLoadingMultipleRestCompatibilityPlugins() throws IOException { plugins.add(TestRestCompatibility1.class); try (Node node = new MockNode(settings.build(), plugins)) { - List compatibleNamedXContents = node.getCompatibleNamedXContents(); + List compatibleNamedXContents = node.getCompatibleNamedXContents() + .collect(Collectors.toList()); assertThat(compatibleNamedXContents, contains(v7CompatibleEntries)); } } @@ -396,7 +399,8 @@ public void testLoadingMultipleRestCompatibilityPlugins() throws IOException { plugins.add(TestRestCompatibility1.class); try (Node node = new MockNode(settings.build(), plugins)) { - List compatibleNamedXContents = node.getCompatibleNamedXContents(); + List compatibleNamedXContents = node.getCompatibleNamedXContents() + .collect(Collectors.toList());; assertThat(compatibleNamedXContents, contains(v8CompatibleEntries)); } } diff --git a/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java b/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java index 05c965855a7fd..7fe88914f4a18 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java @@ -8,6 +8,7 @@ package org.elasticsearch.search; import org.apache.lucene.search.IndexSearcher; +import org.apache.lucene.search.Query; import org.apache.lucene.util.CharsRefBuilder; import org.elasticsearch.common.CheckedBiConsumer; import org.elasticsearch.common.io.stream.StreamInput; @@ -15,8 +16,11 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.common.xcontent.ParseField; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.core.RestApiVersion; +import org.elasticsearch.index.query.AbstractQueryBuilder; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryRewriteContext; import org.elasticsearch.index.query.SearchExecutionContext; @@ -167,6 +171,7 @@ public List> getRescorers() { expectThrows(IllegalArgumentException.class, registryForPlugin(registersDupeRescorer)); } + private ThrowingRunnable registryForPlugin(SearchPlugin plugin) { return () -> new NamedXContentRegistry(new SearchModule(Settings.EMPTY, singletonList(plugin)) .getNamedXContents()); @@ -607,4 +612,73 @@ public String getWriteableName() { return "test"; } } + + static class CompatQueryBuilder extends AbstractQueryBuilder { + public static final String NAME = "compat_name"; + public static final ParseField NAME_V7 = new ParseField(NAME) + .forRestApiVersion(RestApiVersion.equalTo(RestApiVersion.minimumSupported())); + + public static CompatQueryBuilder fromXContent(XContentParser parser) throws IOException { + return null; + } + + @Override + public String getWriteableName() { + return NAME; + } + + @Override + protected void doWriteTo(StreamOutput out) throws IOException { + } + + @Override + protected void doXContent(XContentBuilder builder, Params params) throws IOException { + } + + @Override + protected Query doToQuery(SearchExecutionContext context) throws IOException { + return null; + } + + @Override + protected boolean doEquals(CompatQueryBuilder other) { + return false; + } + + @Override + protected int doHashCode() { + return 0; + } + }; + + public void testRegisterRestApiCompatibleQuery() { + SearchPlugin registerCompatQuery = new SearchPlugin() { + @Override + public List> getQueries() { + return singletonList(new QuerySpec<>(CompatQueryBuilder.NAME_V7, + (streamInput)-> new CompatQueryBuilder(), CompatQueryBuilder::fromXContent)); + } + }; + + final SearchModule searchModule = new SearchModule(Settings.EMPTY, singletonList(registerCompatQuery)); + + // all entries can be used for current and previous versions except for compatible entry + assertThat(searchModule.getNamedXContents().stream() + .filter(e -> + // filter out compatible entry + e.name.match(CompatQueryBuilder.NAME_V7.getPreferredName(), LoggingDeprecationHandler.INSTANCE) == false) + .filter(e -> RestApiVersion.minimumSupported().matches(e.restApiCompatibility)) + .filter(e -> RestApiVersion.current().matches(e.restApiCompatibility)) + .collect(toSet()), + hasSize(searchModule.getNamedXContents().size() -1 )); + + + final List compatEntry = searchModule.getNamedXContents().stream() + .filter(e -> e.categoryClass.equals(QueryBuilder.class) && + e.name.match(CompatQueryBuilder.NAME_V7.getPreferredName(), LoggingDeprecationHandler.INSTANCE)) + .collect(toList()); + assertThat(compatEntry, hasSize(1)); + assertTrue(RestApiVersion.minimumSupported().matches(compatEntry.get(0).restApiCompatibility)); + assertFalse(RestApiVersion.current().matches(compatEntry.get(0).restApiCompatibility)); + } } From deaee55c9ffde53fcc77da2fadb96151c45f519e Mon Sep 17 00:00:00 2001 From: pgomulka Date: Thu, 29 Jul 2021 17:01:19 +0200 Subject: [PATCH 2/8] remove getNamedXContentForCompatibility --- .../xcontent/NamedXContentRegistry.java | 14 ++- .../java/org/elasticsearch/node/Node.java | 11 +- .../org/elasticsearch/plugins/Plugin.java | 9 -- .../CompatibleNamedXContentRegistryTests.java | 49 ++++---- .../org/elasticsearch/node/NodeTests.java | 112 +++++++++--------- 5 files changed, 95 insertions(+), 100 deletions(-) diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/NamedXContentRegistry.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/NamedXContentRegistry.java index c12bf8cb4a98e..6e6e40223761a 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/NamedXContentRegistry.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/NamedXContentRegistry.java @@ -51,8 +51,7 @@ public static class Entry { * Creates a new entry which can be stored by the registry. */ public Entry(Class categoryClass, ParseField name, CheckedFunction parser) { - this(categoryClass, name, (p, c) -> parser.apply(p), - RestApiVersion.onOrAfter(RestApiVersion.minimumSupported())); + this(categoryClass, name, (p, c) -> parser.apply(p), name.getForRestApiVersion()); } public Entry(Class categoryClass, ParseField name, CheckedFunction parser, @@ -64,8 +63,7 @@ public Entry(Class categoryClass, ParseField name, CheckedFunction Entry(Class categoryClass, ParseField name, ContextParser parser) { - this(categoryClass, name, parser, - RestApiVersion.onOrAfter(RestApiVersion.minimumSupported())); + this(categoryClass, name, parser, name.getForRestApiVersion()); } public Entry(Class categoryClass, ParseField name, ContextParser parser, @@ -128,6 +126,12 @@ private void registerParsers(Map, Map T parseNamedObject(Class categoryClass, String name, XContentParser parser, C context) throws IOException { + Entry entry = lookupParser(categoryClass, name, parser); + return categoryClass.cast(entry.parser.parse(parser, context)); + } + + //scope for testing + public Entry lookupParser(Class categoryClass, String name, XContentParser parser) { Map parsers = registry.getOrDefault(parser.getRestApiVersion(), emptyMap()) .get(categoryClass); if (parsers == null) { @@ -147,7 +151,7 @@ public T parseNamedObject(Class categoryClass, String name, XContentPa throw new XContentParseException(parser.getTokenLocation(), "unable to parse " + categoryClass.getSimpleName() + " with name [" + name + "]: parser didn't match"); } - return categoryClass.cast(entry.parser.parse(parser, context)); + return entry; } } diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index a19b5920cc622..6075cc2eea268 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -255,7 +255,9 @@ public class Node implements Closeable { private final Collection pluginLifecycleComponents; private final LocalNodeFactory localNodeFactory; private final NodeService nodeService; + // for testing final NamedWriteableRegistry namedWriteableRegistry; + final NamedXContentRegistry namedXContentRegistry; public Node(Environment environment) { this(environment, Collections.emptyList(), true); @@ -416,8 +418,7 @@ protected Node(final Environment initialEnvironment, searchModule.getNamedXContents().stream(), pluginsService.filterPlugins(Plugin.class).stream() .flatMap(p -> p.getNamedXContent().stream()), - ClusterModule.getNamedXWriteables().stream(), - getCompatibleNamedXContents()) + ClusterModule.getNamedXWriteables().stream()) .flatMap(Function.identity()).collect(toList()) ); final Map featuresMap = pluginsService @@ -722,6 +723,7 @@ protected Node(final Environment initialEnvironment, transportService.getRemoteClusterService(), namedWriteableRegistry); this.namedWriteableRegistry = namedWriteableRegistry; + this.namedXContentRegistry = xContentRegistry; logger.debug("initializing HTTP handlers ..."); actionModule.initRestHandlers(() -> clusterService.state().nodes()); @@ -737,11 +739,6 @@ protected Node(final Environment initialEnvironment, } } - // package scope for testing - Stream getCompatibleNamedXContents() { - return pluginsService.filterPlugins(Plugin.class).stream() - .flatMap(p -> p.getNamedXContentForCompatibility().stream()); - } protected TransportService newTransportService(Settings settings, Transport transport, ThreadPool threadPool, TransportInterceptor interceptor, diff --git a/server/src/main/java/org/elasticsearch/plugins/Plugin.java b/server/src/main/java/org/elasticsearch/plugins/Plugin.java index cb22132589a6d..f39d628721f02 100644 --- a/server/src/main/java/org/elasticsearch/plugins/Plugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/Plugin.java @@ -112,15 +112,6 @@ public List getNamedXContent() { return Collections.emptyList(); } - /** - * Returns parsers with compatible logic for named objects this plugin will parse from - * {@link XContentParser#namedObject(Class, String, Object)}. - * @see NamedWriteableRegistry - */ - public List getNamedXContentForCompatibility() { - return Collections.emptyList(); - } - /** * Called before a new index is created on a node. The given module can be used to register index-level * extensions. diff --git a/server/src/test/java/org/elasticsearch/common/xcontent/CompatibleNamedXContentRegistryTests.java b/server/src/test/java/org/elasticsearch/common/xcontent/CompatibleNamedXContentRegistryTests.java index 207e0edec22b8..2393ddf233c90 100644 --- a/server/src/test/java/org/elasticsearch/common/xcontent/CompatibleNamedXContentRegistryTests.java +++ b/server/src/test/java/org/elasticsearch/common/xcontent/CompatibleNamedXContentRegistryTests.java @@ -19,6 +19,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.function.Function; import static org.hamcrest.core.IsEqual.equalTo; @@ -30,18 +31,18 @@ public class CompatibleNamedXContentRegistryTests extends ESTestCase { static class ParentObject { private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>("parentParser", false, - (a, name) -> new ParentObject(name, (SubObject) a[0])); + (a, name) -> new ParentObject(name, (NewSubObject) a[0])); String name; - SubObject subObject; + NewSubObject subObject; static { PARSER.declareNamedObject(ConstructingObjectParser.constructorArg(), - (p, c, n) -> p.namedObject(SubObject.class, n, null), + (p, c, n) -> p.namedObject(NewSubObject.class, n, null), new ParseField("subObject")); } - ParentObject(String name, SubObject subObject) { + ParentObject(String name, NewSubObject subObject) { this.name = name; this.subObject = subObject; } @@ -52,10 +53,11 @@ public static ParentObject parse(XContentParser parser) { } } - static class SubObject { - private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( + static class NewSubObject { + public static final Function REST_API_VERSION = RestApiVersion.onOrAfter(RestApiVersion.current()); + private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( "parser1", false, - a -> new SubObject((String) a[0])); + a -> new NewSubObject((String) a[0])); static final ParseField NAME = new ParseField("namedObjectName1") .forRestApiVersion(RestApiVersion.onOrAfter(RestApiVersion.current())); @@ -65,40 +67,43 @@ static class SubObject { String field; - SubObject(String field) { + NewSubObject(String field) { this.field = field; } - public static SubObject parse(XContentParser parser) { + public static NewSubObject parse(XContentParser parser) { return PARSER.apply(parser, null); } } - static class OldSubObject extends SubObject { - private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( + static class OldSubObject { + public static final Function REST_API_VERSION = RestApiVersion.equalTo(RestApiVersion.minimumSupported()); + + private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( "parser2", false, - a -> new SubObject((String) a[0])); + a -> new NewSubObject((String) a[0])); static final ParseField NAME = new ParseField("namedObjectName1") .forRestApiVersion(RestApiVersion.equalTo(RestApiVersion.minimumSupported())); static { PARSER.declareString(ConstructingObjectParser.constructorArg(), new ParseField("old_field")); } + String field; OldSubObject(String field) { - super(field); + this.field = field; } - public static SubObject parse(XContentParser parser) { + public static NewSubObject parse(XContentParser parser) { return PARSER.apply(parser, null); } } public void testNotCompatibleRequest() throws IOException { NamedXContentRegistry registry = new NamedXContentRegistry( - List.of(new NamedXContentRegistry.Entry(SubObject.class, SubObject.NAME, SubObject::parse, - SubObject.NAME.getForRestApiVersion()), - new NamedXContentRegistry.Entry(SubObject.class, OldSubObject.NAME, OldSubObject::parse, - OldSubObject.NAME.getForRestApiVersion()))); + List.of(new NamedXContentRegistry.Entry(NewSubObject.class, NewSubObject.NAME, NewSubObject::parse, + NewSubObject.REST_API_VERSION), + new NamedXContentRegistry.Entry(NewSubObject.class, OldSubObject.NAME, OldSubObject::parse, + OldSubObject.REST_API_VERSION))); XContentBuilder b = XContentBuilder.builder(XContentType.JSON.xContent()); b.startObject(); @@ -128,10 +133,10 @@ public void testNotCompatibleRequest() throws IOException { public void testCompatibleRequest() throws IOException { NamedXContentRegistry registry = new NamedXContentRegistry( - List.of(new NamedXContentRegistry.Entry(SubObject.class, SubObject.NAME, SubObject::parse, - SubObject.NAME.getForRestApiVersion()), - new NamedXContentRegistry.Entry(SubObject.class, OldSubObject.NAME, OldSubObject::parse, - OldSubObject.NAME.getForRestApiVersion()))); + List.of(new NamedXContentRegistry.Entry(NewSubObject.class, NewSubObject.NAME, NewSubObject::parse, + NewSubObject.REST_API_VERSION), + new NamedXContentRegistry.Entry(NewSubObject.class, OldSubObject.NAME, OldSubObject::parse, + OldSubObject.REST_API_VERSION))); XContentBuilder b = XContentBuilder.builder(XContentType.JSON.xContent()); b.startObject(); diff --git a/server/src/test/java/org/elasticsearch/node/NodeTests.java b/server/src/test/java/org/elasticsearch/node/NodeTests.java index b97b8dc33fe40..48b8d1bd5eb77 100644 --- a/server/src/test/java/org/elasticsearch/node/NodeTests.java +++ b/server/src/test/java/org/elasticsearch/node/NodeTests.java @@ -13,12 +13,18 @@ import org.elasticsearch.bootstrap.BootstrapContext; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.common.breaker.CircuitBreaker; +import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.network.NetworkModule; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.transport.BoundTransportAddress; import org.elasticsearch.common.xcontent.ContextParser; +import org.elasticsearch.common.xcontent.MediaType; +import org.elasticsearch.common.xcontent.NamedObjectNotFoundException; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.ParseField; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.env.Environment; import org.elasticsearch.index.IndexService; @@ -29,28 +35,28 @@ import org.elasticsearch.indices.breaker.CircuitBreakerService; import org.elasticsearch.plugins.CircuitBreakerPlugin; import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.rest.RestRequest; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.InternalTestCluster; import org.elasticsearch.test.MockHttpTransport; +import org.elasticsearch.test.rest.FakeRestRequest; import org.elasticsearch.threadpool.ThreadPool; -import org.mockito.Mockito; import java.io.IOException; import java.nio.file.Path; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.concurrent.CountDownLatch; import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.stream.Collectors; import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS; import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS; import static org.elasticsearch.test.NodeRoles.dataNode; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; -import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; @@ -337,74 +343,66 @@ public void setCircuitBreaker(CircuitBreaker circuitBreaker) { } } - - interface MockRestApiVersion { - RestApiVersion minimumRestCompatibilityVersion(); - } - - static MockRestApiVersion MockCompatibleVersion = mock(MockRestApiVersion.class); - - static NamedXContentRegistry.Entry v7CompatibleEntries = new NamedXContentRegistry.Entry(Integer.class, - new ParseField("name"), mock(ContextParser.class)); - static NamedXContentRegistry.Entry v8CompatibleEntries = new NamedXContentRegistry.Entry(Integer.class, - new ParseField("name2"), mock(ContextParser.class)); + static NamedXContentRegistry.Entry compatibleEntries = new NamedXContentRegistry.Entry(Integer.class, + new ParseField("name").forRestApiVersion(RestApiVersion.equalTo(RestApiVersion.minimumSupported())), mock(ContextParser.class)); + static NamedXContentRegistry.Entry currentVersionEntries = new NamedXContentRegistry.Entry(Integer.class, + new ParseField("name2").forRestApiVersion(RestApiVersion.onOrAfter(RestApiVersion.minimumSupported())), mock(ContextParser.class)); public static class TestRestCompatibility1 extends Plugin { - @Override - public List getNamedXContentForCompatibility() { - // real plugin will use CompatibleVersion.minimumRestCompatibilityVersion() - if (/*RestApiVersion.minimumSupported() == */ - MockCompatibleVersion.minimumRestCompatibilityVersion().equals(RestApiVersion.V_7)) { - //return set of N-1 entries - return List.of(v7CompatibleEntries); - } - // after major release, new compatible apis can be added before the old ones are removed. - if (/*RestApiVersion.minimumSupported() == */ - MockCompatibleVersion.minimumRestCompatibilityVersion().equals(RestApiVersion.V_8)) { - return List.of(v8CompatibleEntries); + public List getNamedXContent() { + return List.of(compatibleEntries); + } + } - } - return super.getNamedXContentForCompatibility(); + public static class TestRestCompatibility2 extends Plugin { + @Override + public List getNamedXContent() { + return List.of(currentVersionEntries); } } - // This test shows an example on how multiple compatible namedxcontent can be present at the same time. + // This test shows an example on how multiple plugins register namedXContent entries public void testLoadingMultipleRestCompatibilityPlugins() throws IOException { + Settings.Builder settings = baseSettings(); + List> plugins = basePlugins(); + plugins.add(TestRestCompatibility1.class); + plugins.add(TestRestCompatibility2.class); - Mockito.when(MockCompatibleVersion.minimumRestCompatibilityVersion()) - .thenReturn(RestApiVersion.V_7); - - { - Settings.Builder settings = baseSettings(); + try (Node node = new MockNode(settings.build(), plugins)) { + final NamedXContentRegistry namedXContentRegistry = node.namedXContentRegistry; + RestRequest compatibleRequest = request(namedXContentRegistry, RestApiVersion.minimumSupported()); + try (XContentParser p = compatibleRequest.contentParser()) { + NamedXContentRegistry.Entry field = namedXContentRegistry.lookupParser(Integer.class, "name", p); + assertTrue(RestApiVersion.minimumSupported().matches(field.restApiCompatibility)); + + field = namedXContentRegistry.lookupParser(Integer.class, "name2", p); + assertTrue(RestApiVersion.current().matches(field.restApiCompatibility)); + } - // throw an exception when two plugins are registered - List> plugins = basePlugins(); - plugins.add(TestRestCompatibility1.class); + RestRequest currentRequest = request(namedXContentRegistry, RestApiVersion.current()); + try (XContentParser p = currentRequest.contentParser()) { + NamedXContentRegistry.Entry field = namedXContentRegistry.lookupParser(Integer.class, "name2", p); + assertTrue(RestApiVersion.minimumSupported().matches(field.restApiCompatibility)); - try (Node node = new MockNode(settings.build(), plugins)) { - List compatibleNamedXContents = node.getCompatibleNamedXContents() - .collect(Collectors.toList()); - assertThat(compatibleNamedXContents, contains(v7CompatibleEntries)); - } - } - // after version bump CompatibleVersion.minimumRestCompatibilityVersion() will return V_8 - Mockito.when(MockCompatibleVersion.minimumRestCompatibilityVersion()) - .thenReturn(RestApiVersion.V_8); - { - Settings.Builder settings = baseSettings(); - - // throw an exception when two plugins are registered - List> plugins = basePlugins(); - plugins.add(TestRestCompatibility1.class); - - try (Node node = new MockNode(settings.build(), plugins)) { - List compatibleNamedXContents = node.getCompatibleNamedXContents() - .collect(Collectors.toList());; - assertThat(compatibleNamedXContents, contains(v8CompatibleEntries)); + expectThrows(NamedObjectNotFoundException.class, () -> namedXContentRegistry.lookupParser(Integer.class, "name", p)); } } } + private RestRequest request(NamedXContentRegistry namedXContentRegistry, RestApiVersion restApiVersion) throws IOException { + String mediaType = XContentType.VND_JSON.toParsedMediaType() + .responseContentTypeHeader(Map.of(MediaType.COMPATIBLE_WITH_PARAMETER_NAME, + String.valueOf(restApiVersion.major))); + List mediaTypeList = Collections.singletonList(mediaType); + XContentBuilder b = XContentBuilder.builder(XContentType.JSON.xContent()).startObject().endObject(); + + return new FakeRestRequest.Builder(namedXContentRegistry) + .withContent(BytesReference.bytes(b), RestRequest.parseContentType(mediaTypeList)) + .withPath("/foo") + .withHeaders(Map.of("Content-Type", mediaTypeList, "Accept", mediaTypeList)) + .build(); + } + } From 8b6452ba36494d75dd07be0b335ae236a3837dc9 Mon Sep 17 00:00:00 2001 From: pgomulka Date: Fri, 30 Jul 2021 17:15:02 +0200 Subject: [PATCH 3/8] draft common term query --- rest-api-spec/build.gradle | 4 +- .../10_multi_match_cutoff_frequency.yml | 100 ++++++++++++++++++ .../index/query/CommonTermsQueryBuilder.java | 63 +++++++++++ .../index/query/MatchQueryBuilder.java | 11 +- .../index/query/MultiMatchQueryBuilder.java | 12 ++- .../elasticsearch/search/SearchModule.java | 8 ++ 6 files changed, 194 insertions(+), 4 deletions(-) create mode 100644 rest-api-spec/src/yamlRestCompatTest/resources/rest-api-spec/test/v7compat/search/10_multi_match_cutoff_frequency.yml create mode 100644 server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java diff --git a/rest-api-spec/build.gradle b/rest-api-spec/build.gradle index 85d4adddae9d2..a973e0bedb0f9 100644 --- a/rest-api-spec/build.gradle +++ b/rest-api-spec/build.gradle @@ -81,6 +81,9 @@ def v7compatibilityNotSupportedTests = { 'field_caps/30_filter/Field caps with index filter', //behaviour change after #63692 4digits dates are parsed as epoch and in quotes as year 'indices.forcemerge/10_basic/Check deprecation warning when incompatible only_expunge_deletes and max_num_segments values are both set', //#44761 bug fix + + 'search/310_match_bool_prefix/multi_match multiple fields with cutoff_frequency throws exception', //#42654 cutoff_frequency, common terms are not supported. Throwing an exception + ] } tasks.named("yamlRestCompatTest").configure { @@ -90,7 +93,6 @@ tasks.named("yamlRestCompatTest").configure { } systemProperty 'tests.rest.blacklist', ([ 'search.aggregation/200_top_hits_metric/top_hits aggregation with sequence numbers', - 'search/310_match_bool_prefix/multi_match multiple fields with cutoff_frequency throws exception', //cutoff_frequency 'search/340_type_query/type query', // type_query - probably should behave like match_all ] + v7compatibilityNotSupportedTests()) .join(',') diff --git a/rest-api-spec/src/yamlRestCompatTest/resources/rest-api-spec/test/v7compat/search/10_multi_match_cutoff_frequency.yml b/rest-api-spec/src/yamlRestCompatTest/resources/rest-api-spec/test/v7compat/search/10_multi_match_cutoff_frequency.yml new file mode 100644 index 0000000000000..95f5784fbf631 --- /dev/null +++ b/rest-api-spec/src/yamlRestCompatTest/resources/rest-api-spec/test/v7compat/search/10_multi_match_cutoff_frequency.yml @@ -0,0 +1,100 @@ +--- +setup: +- skip: + version: " - 7.1.99" + reason: "added in 7.2.0" + features: + - "headers" + - "allowed_warnings_regex" +- do: + indices.create: + index: "test" + body: + mappings: + properties: + my_field1: + type: "text" + my_field2: + type: "text" + headers: + Content-Type: "application/vnd.elasticsearch+json;compatible-with=7" + Accept: "application/vnd.elasticsearch+json;compatible-with=7" + allowed_warnings_regex: + - "\\[types removal\\].*" +- do: + index: + index: "test" + id: 1 + body: + my_field1: "brown fox jump" + my_field2: "xylophone" + headers: + Content-Type: "application/vnd.elasticsearch+json;compatible-with=7" + Accept: "application/vnd.elasticsearch+json;compatible-with=7" + allowed_warnings_regex: + - "\\[types removal\\].*" +- do: + index: + index: "test" + id: 2 + body: + my_field1: "brown emu jump" + my_field2: "xylophone" + headers: + Content-Type: "application/vnd.elasticsearch+json;compatible-with=7" + Accept: "application/vnd.elasticsearch+json;compatible-with=7" + allowed_warnings_regex: + - "\\[types removal\\].*" +- do: + index: + index: "test" + id: 3 + body: + my_field1: "jumparound" + my_field2: "emu" + headers: + Content-Type: "application/vnd.elasticsearch+json;compatible-with=7" + Accept: "application/vnd.elasticsearch+json;compatible-with=7" + allowed_warnings_regex: + - "\\[types removal\\].*" +- do: + index: + index: "test" + id: 4 + body: + my_field1: "dog" + my_field2: "brown fox jump lazy" + headers: + Content-Type: "application/vnd.elasticsearch+json;compatible-with=7" + Accept: "application/vnd.elasticsearch+json;compatible-with=7" + allowed_warnings_regex: + - "\\[types removal\\].*" +- do: + indices.refresh: {} + headers: + Content-Type: "application/vnd.elasticsearch+json;compatible-with=7" + Accept: "application/vnd.elasticsearch+json;compatible-with=7" + allowed_warnings_regex: + - "\\[types removal\\].*" + +--- +multi_match multiple fields with cutoff_frequency throws exception: +- do: + catch: "/cutoff_freqency is not supported. The [match] query can skip block of documents efficiently if the total number of hits is not tracked/" + search: + rest_total_hits_as_int: true + index: "test" + body: + query: + multi_match: + query: "brown" + type: "bool_prefix" + fields: + - "my_field1" + - "my_field2" + cutoff_frequency: 0.001 + headers: + Content-Type: "application/vnd.elasticsearch+json;compatible-with=7" + Accept: "application/vnd.elasticsearch+json;compatible-with=7" + allowed_warnings_regex: + - "\\[types removal\\].*" diff --git a/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java new file mode 100644 index 0000000000000..a3a77e810d4a8 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java @@ -0,0 +1,63 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.index.query; + +import org.apache.lucene.search.Query; +import org.elasticsearch.common.ParsingException; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.logging.DeprecationLogger; +import org.elasticsearch.common.xcontent.ParseField; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.core.RestApiVersion; + +import java.io.IOException; + +public class CommonTermsQueryBuilder extends AbstractQueryBuilder { + private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(CommonTermsQueryBuilder.class); + public static final String COMMON_TERMS_QUERY_DEPRECATION_MSG = "[match] query which can efficiently " + + "skip blocks of documents if the total number of hits is not tracked. Common Terms Query usage is not supported."; + + public static ParseField NAME = new ParseField("common") + .withAllDeprecated(COMMON_TERMS_QUERY_DEPRECATION_MSG) + .forRestApiVersion(RestApiVersion.equalTo(RestApiVersion.V_7)); + + @Override + protected void doWriteTo(StreamOutput out) throws IOException { + } + + @Override + protected void doXContent(XContentBuilder builder, Params params) throws IOException { + } + + @Override + protected Query doToQuery(SearchExecutionContext context) throws IOException { + return null; + } + + @Override + protected boolean doEquals(CommonTermsQueryBuilder other) { + return false; + } + + @Override + protected int doHashCode() { + return 0; + } + + @Override + public String getWriteableName() { + return null; + } + public static CommonTermsQueryBuilder fromXContent(XContentParser parser) throws IOException { + deprecationLogger.compatibleApiWarning("common_term_query", COMMON_TERMS_QUERY_DEPRECATION_MSG); + throw new ParsingException(parser.getTokenLocation(), COMMON_TERMS_QUERY_DEPRECATION_MSG); + } + +} diff --git a/server/src/main/java/org/elasticsearch/index/query/MatchQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/MatchQueryBuilder.java index dcc124b86a868..724932175914d 100644 --- a/server/src/main/java/org/elasticsearch/index/query/MatchQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/MatchQueryBuilder.java @@ -20,6 +20,7 @@ import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.index.query.support.QueryParsers; import org.elasticsearch.index.search.MatchQueryParser; @@ -31,7 +32,12 @@ * result of the analysis. */ public class MatchQueryBuilder extends AbstractQueryBuilder { - + private static final String CUTOFF_FREQUENCY_DEPRECATION_MSG = "cutoff_freqency is not supported. " + + "The [match] query can skip block of documents efficiently if the total number of hits is not tracked"; + public static final ParseField CUTOFF_FREQUENCY_FIELD = + new ParseField("cutoff_frequency") + .withAllDeprecated(CUTOFF_FREQUENCY_DEPRECATION_MSG) + .forRestApiVersion(RestApiVersion.equalTo(RestApiVersion.V_7)); public static final ParseField ZERO_TERMS_QUERY_FIELD = new ParseField("zero_terms_query"); public static final ParseField LENIENT_FIELD = new ParseField("lenient"); public static final ParseField FUZZY_TRANSPOSITIONS_FIELD = new ParseField("fuzzy_transpositions"); @@ -464,6 +470,9 @@ public static MatchQueryBuilder fromXContent(XContentParser parser) throws IOExc queryName = parser.text(); } else if (GENERATE_SYNONYMS_PHRASE_QUERY.match(currentFieldName, parser.getDeprecationHandler())) { autoGenerateSynonymsPhraseQuery = parser.booleanValue(); + } else if (parser.getRestApiVersion() == RestApiVersion.V_7 && + CUTOFF_FREQUENCY_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { + throw new ParsingException(parser.getTokenLocation(), CUTOFF_FREQUENCY_DEPRECATION_MSG); } else { throw new ParsingException(parser.getTokenLocation(), "[" + NAME + "] query does not support [" + currentFieldName + "]"); diff --git a/server/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java index b1b00114fe5f8..a7ac3f63f43d6 100644 --- a/server/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java @@ -23,6 +23,7 @@ import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.index.query.support.QueryParsers; import org.elasticsearch.index.search.MatchQueryParser; import org.elasticsearch.index.search.MultiMatchQueryParser; @@ -42,7 +43,12 @@ public class MultiMatchQueryBuilder extends AbstractQueryBuilder { public static final String NAME = "multi_match"; - + private static final String CUTOFF_FREQUENCY_DEPRECATION_MSG = "you can omit this option, " + + "the [multi_match] query can skip block of documents efficiently if the total number of hits is not tracked"; + private static final ParseField CUTOFF_FREQUENCY_FIELD = + new ParseField("cutoff_frequency") + .withAllDeprecated(CUTOFF_FREQUENCY_DEPRECATION_MSG) + .forRestApiVersion(RestApiVersion.equalTo(RestApiVersion.V_7)); public static final MultiMatchQueryBuilder.Type DEFAULT_TYPE = MultiMatchQueryBuilder.Type.BEST_FIELDS; public static final Operator DEFAULT_OPERATOR = Operator.OR; public static final int DEFAULT_PHRASE_SLOP = MatchQueryParser.DEFAULT_PHRASE_SLOP; @@ -570,7 +576,6 @@ public static MultiMatchQueryBuilder fromXContent(XContentParser parser) throws ZeroTermsQueryOption zeroTermsQuery = DEFAULT_ZERO_TERMS_QUERY; boolean autoGenerateSynonymsPhraseQuery = true; boolean fuzzyTranspositions = DEFAULT_FUZZY_TRANSPOSITIONS; - float boost = AbstractQueryBuilder.DEFAULT_BOOST; String queryName = null; @@ -633,6 +638,9 @@ public static MultiMatchQueryBuilder fromXContent(XContentParser parser) throws autoGenerateSynonymsPhraseQuery = parser.booleanValue(); } else if (FUZZY_TRANSPOSITIONS_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { fuzzyTranspositions = parser.booleanValue(); + } else if (parser.getRestApiVersion() == RestApiVersion.V_7 && + CUTOFF_FREQUENCY_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { + throw new ParsingException(parser.getTokenLocation(), CUTOFF_FREQUENCY_DEPRECATION_MSG); } else { throw new ParsingException(parser.getTokenLocation(), "[" + NAME + "] query does not support [" + currentFieldName + "]"); diff --git a/server/src/main/java/org/elasticsearch/search/SearchModule.java b/server/src/main/java/org/elasticsearch/search/SearchModule.java index e6d98d64d5519..a3b215e1c7f2a 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchModule.java +++ b/server/src/main/java/org/elasticsearch/search/SearchModule.java @@ -23,9 +23,11 @@ import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.core.Nullable; +import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.index.query.BoolQueryBuilder; import org.elasticsearch.index.query.BoostingQueryBuilder; import org.elasticsearch.index.query.CombinedFieldsQueryBuilder; +import org.elasticsearch.index.query.CommonTermsQueryBuilder; import org.elasticsearch.index.query.ConstantScoreQueryBuilder; import org.elasticsearch.index.query.DisMaxQueryBuilder; import org.elasticsearch.index.query.DistanceFeatureQueryBuilder; @@ -480,6 +482,12 @@ private ValuesSourceRegistry registerAggregations(List plugins) { .setAggregatorRegistrar(CompositeAggregationBuilder::registerAggregators), builder ); + + if(RestApiVersion.minimumSupported() == RestApiVersion.V_7) { + registerQuery(new QuerySpec<>(CommonTermsQueryBuilder.NAME, + (streamInput) -> new CommonTermsQueryBuilder(), CommonTermsQueryBuilder::fromXContent)); + } + registerFromPlugin(plugins, SearchPlugin::getAggregations, (agg) -> this.registerAggregation(agg, builder)); // after aggs have been registered, see if there are any new VSTypes that need to be linked to core fields From 512de4b59e6d162bbade5d31727d57d5046aa770 Mon Sep 17 00:00:00 2001 From: pgomulka Date: Mon, 2 Aug 2021 10:17:37 +0200 Subject: [PATCH 4/8] tests --- .../10_multi_match_cutoff_frequency.yml | 154 +++++++++--------- .../search/SearchModuleTests.java | 11 +- 2 files changed, 85 insertions(+), 80 deletions(-) diff --git a/rest-api-spec/src/yamlRestCompatTest/resources/rest-api-spec/test/v7compat/search/10_multi_match_cutoff_frequency.yml b/rest-api-spec/src/yamlRestCompatTest/resources/rest-api-spec/test/v7compat/search/10_multi_match_cutoff_frequency.yml index 95f5784fbf631..cd69115a9acb1 100644 --- a/rest-api-spec/src/yamlRestCompatTest/resources/rest-api-spec/test/v7compat/search/10_multi_match_cutoff_frequency.yml +++ b/rest-api-spec/src/yamlRestCompatTest/resources/rest-api-spec/test/v7compat/search/10_multi_match_cutoff_frequency.yml @@ -1,86 +1,86 @@ --- setup: -- skip: - version: " - 7.1.99" - reason: "added in 7.2.0" - features: - - "headers" - - "allowed_warnings_regex" -- do: - indices.create: - index: "test" - body: - mappings: - properties: - my_field1: - type: "text" - my_field2: - type: "text" - headers: - Content-Type: "application/vnd.elasticsearch+json;compatible-with=7" - Accept: "application/vnd.elasticsearch+json;compatible-with=7" - allowed_warnings_regex: - - "\\[types removal\\].*" -- do: - index: - index: "test" - id: 1 - body: - my_field1: "brown fox jump" - my_field2: "xylophone" - headers: - Content-Type: "application/vnd.elasticsearch+json;compatible-with=7" - Accept: "application/vnd.elasticsearch+json;compatible-with=7" - allowed_warnings_regex: - - "\\[types removal\\].*" -- do: - index: - index: "test" - id: 2 - body: - my_field1: "brown emu jump" - my_field2: "xylophone" - headers: - Content-Type: "application/vnd.elasticsearch+json;compatible-with=7" - Accept: "application/vnd.elasticsearch+json;compatible-with=7" - allowed_warnings_regex: - - "\\[types removal\\].*" -- do: - index: - index: "test" - id: 3 - body: - my_field1: "jumparound" - my_field2: "emu" - headers: - Content-Type: "application/vnd.elasticsearch+json;compatible-with=7" - Accept: "application/vnd.elasticsearch+json;compatible-with=7" - allowed_warnings_regex: - - "\\[types removal\\].*" -- do: - index: - index: "test" - id: 4 - body: - my_field1: "dog" - my_field2: "brown fox jump lazy" - headers: - Content-Type: "application/vnd.elasticsearch+json;compatible-with=7" - Accept: "application/vnd.elasticsearch+json;compatible-with=7" - allowed_warnings_regex: - - "\\[types removal\\].*" -- do: - indices.refresh: {} - headers: - Content-Type: "application/vnd.elasticsearch+json;compatible-with=7" - Accept: "application/vnd.elasticsearch+json;compatible-with=7" - allowed_warnings_regex: - - "\\[types removal\\].*" + - skip: + version: "9.0.0 - " + reason: "compatible from 8.x to 7.x" + features: + - "headers" + - "allowed_warnings_regex" + - do: + indices.create: + index: "test" + body: + mappings: + properties: + my_field1: + type: "text" + my_field2: + type: "text" + headers: + Content-Type: "application/vnd.elasticsearch+json;compatible-with=7" + Accept: "application/vnd.elasticsearch+json;compatible-with=7" + allowed_warnings_regex: + - "\\[types removal\\].*" + - do: + index: + index: "test" + id: 1 + body: + my_field1: "brown fox jump" + my_field2: "xylophone" + headers: + Content-Type: "application/vnd.elasticsearch+json;compatible-with=7" + Accept: "application/vnd.elasticsearch+json;compatible-with=7" + allowed_warnings_regex: + - "\\[types removal\\].*" + - do: + index: + index: "test" + id: 2 + body: + my_field1: "brown emu jump" + my_field2: "xylophone" + headers: + Content-Type: "application/vnd.elasticsearch+json;compatible-with=7" + Accept: "application/vnd.elasticsearch+json;compatible-with=7" + allowed_warnings_regex: + - "\\[types removal\\].*" + - do: + index: + index: "test" + id: 3 + body: + my_field1: "jumparound" + my_field2: "emu" + headers: + Content-Type: "application/vnd.elasticsearch+json;compatible-with=7" + Accept: "application/vnd.elasticsearch+json;compatible-with=7" + allowed_warnings_regex: + - "\\[types removal\\].*" + - do: + index: + index: "test" + id: 4 + body: + my_field1: "dog" + my_field2: "brown fox jump lazy" + headers: + Content-Type: "application/vnd.elasticsearch+json;compatible-with=7" + Accept: "application/vnd.elasticsearch+json;compatible-with=7" + allowed_warnings_regex: + - "\\[types removal\\].*" + - do: + indices.refresh: {} + headers: + Content-Type: "application/vnd.elasticsearch+json;compatible-with=7" + Accept: "application/vnd.elasticsearch+json;compatible-with=7" + allowed_warnings_regex: + - "\\[types removal\\].*" --- multi_match multiple fields with cutoff_frequency throws exception: - do: - catch: "/cutoff_freqency is not supported. The [match] query can skip block of documents efficiently if the total number of hits is not tracked/" + catch: "/cutoff_freqency is not supported. The [match] query can fskip block of documents efficiently if the total number of hits is not tracked/" search: rest_total_hits_as_int: true index: "test" diff --git a/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java b/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java index 7fe88914f4a18..9943864e830a0 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java @@ -21,6 +21,7 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.index.query.AbstractQueryBuilder; +import org.elasticsearch.index.query.CommonTermsQueryBuilder; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryRewriteContext; import org.elasticsearch.index.query.SearchExecutionContext; @@ -242,11 +243,13 @@ public void testRegisteredQueries() { List allSupportedQueries = new ArrayList<>(); Collections.addAll(allSupportedQueries, NON_DEPRECATED_QUERIES); Collections.addAll(allSupportedQueries, DEPRECATED_QUERIES); + Collections.addAll(allSupportedQueries, REST_COMPATIBLE_QUERIES); SearchModule module = new SearchModule(Settings.EMPTY, emptyList()); Set registeredNonDeprecated = module.getNamedXContents().stream() .filter(e -> e.categoryClass.equals(QueryBuilder.class)) .filter(e -> e.name.getAllReplacedWith() == null) + .filter(e -> RestApiVersion.current().matches(e.restApiCompatibility)) .map(e -> e.name.getPreferredName()) .collect(toSet()); Set registeredAll = module.getNamedXContents().stream() @@ -390,6 +393,7 @@ public CheckedBiConsumer getReque //add here deprecated queries to make sure we log a deprecation warnings when they are used private static final String[] DEPRECATED_QUERIES = new String[] {"field_masking_span", "geo_polygon"}; + private static final String[] REST_COMPATIBLE_QUERIES = new String[] {CommonTermsQueryBuilder.NAME.getPreferredName()}; /** * Dummy test {@link AggregationBuilder} used to test registering aggregation builders. @@ -670,14 +674,15 @@ public List> getQueries() { .filter(e -> RestApiVersion.minimumSupported().matches(e.restApiCompatibility)) .filter(e -> RestApiVersion.current().matches(e.restApiCompatibility)) .collect(toSet()), - hasSize(searchModule.getNamedXContents().size() -1 )); + hasSize(searchModule.getNamedXContents().size() - REST_COMPATIBLE_QUERIES.length -1 )); // -1 because of the registered in the test final List compatEntry = searchModule.getNamedXContents().stream() .filter(e -> e.categoryClass.equals(QueryBuilder.class) && - e.name.match(CompatQueryBuilder.NAME_V7.getPreferredName(), LoggingDeprecationHandler.INSTANCE)) + RestApiVersion.minimumSupported().matches(e.name.getForRestApiVersion()) // v7 compatbile + && RestApiVersion.current().matches(e.name.getForRestApiVersion()) == false) // but not v8 compatible .collect(toList()); - assertThat(compatEntry, hasSize(1)); + assertThat(compatEntry, hasSize(REST_COMPATIBLE_QUERIES.length + 1));//+1 because of registered in the test assertTrue(RestApiVersion.minimumSupported().matches(compatEntry.get(0).restApiCompatibility)); assertFalse(RestApiVersion.current().matches(compatEntry.get(0).restApiCompatibility)); } From 4f794c7df92ea7dadef976ecf62dc8f1ca019c4a Mon Sep 17 00:00:00 2001 From: pgomulka Date: Mon, 2 Aug 2021 13:49:52 +0200 Subject: [PATCH 5/8] fix tests and messages --- modules/analysis-common/build.gradle | 8 +- ..._frequency.yml => 10_cutoff_frequency.yml} | 79 ++++++++++--------- .../index/query/CommonTermsQueryBuilder.java | 4 +- .../index/query/MultiMatchQueryBuilder.java | 4 +- .../search/SearchModuleTests.java | 4 +- 5 files changed, 52 insertions(+), 47 deletions(-) rename rest-api-spec/src/yamlRestCompatTest/resources/rest-api-spec/test/v7compat/search/{10_multi_match_cutoff_frequency.yml => 10_cutoff_frequency.yml} (67%) diff --git a/modules/analysis-common/build.gradle b/modules/analysis-common/build.gradle index b1d72e79d19e2..a174356eaa533 100644 --- a/modules/analysis-common/build.gradle +++ b/modules/analysis-common/build.gradle @@ -30,13 +30,11 @@ def v7compatibilityNotSupportedTests = { //marked as not needing compatible api 'indices.analyze/10_analyze/htmlStrip_deprecated', // Cleanup versioned deprecations in analysis #41560 'analysis-common/40_token_filters/delimited_payload_filter_error', //Remove preconfigured delimited_payload_filter #43686 - 'analysis-common/20_analyzers/standard_html_strip' // Cleanup versioned deprecations in analysis #41560 + 'analysis-common/20_analyzers/standard_html_strip', // Cleanup versioned deprecations in analysis #41560 + 'search.query/50_queries_with_synonyms/Test common terms query with stacked tokens', // #42654 - `common` query throws an exception ] } tasks.named("yamlRestCompatTest").configure { - systemProperty 'tests.rest.blacklist', ([ - 'search.query/50_queries_with_synonyms/Test common terms query with stacked tokens', // #42654 - not sure, remove `common` query - ]+ v7compatibilityNotSupportedTests()) - .join(',') + systemProperty 'tests.rest.blacklist', v7compatibilityNotSupportedTests().join(',') } diff --git a/rest-api-spec/src/yamlRestCompatTest/resources/rest-api-spec/test/v7compat/search/10_multi_match_cutoff_frequency.yml b/rest-api-spec/src/yamlRestCompatTest/resources/rest-api-spec/test/v7compat/search/10_cutoff_frequency.yml similarity index 67% rename from rest-api-spec/src/yamlRestCompatTest/resources/rest-api-spec/test/v7compat/search/10_multi_match_cutoff_frequency.yml rename to rest-api-spec/src/yamlRestCompatTest/resources/rest-api-spec/test/v7compat/search/10_cutoff_frequency.yml index cd69115a9acb1..2d645a9419171 100644 --- a/rest-api-spec/src/yamlRestCompatTest/resources/rest-api-spec/test/v7compat/search/10_multi_match_cutoff_frequency.yml +++ b/rest-api-spec/src/yamlRestCompatTest/resources/rest-api-spec/test/v7compat/search/10_cutoff_frequency.yml @@ -33,42 +33,6 @@ setup: Accept: "application/vnd.elasticsearch+json;compatible-with=7" allowed_warnings_regex: - "\\[types removal\\].*" - - do: - index: - index: "test" - id: 2 - body: - my_field1: "brown emu jump" - my_field2: "xylophone" - headers: - Content-Type: "application/vnd.elasticsearch+json;compatible-with=7" - Accept: "application/vnd.elasticsearch+json;compatible-with=7" - allowed_warnings_regex: - - "\\[types removal\\].*" - - do: - index: - index: "test" - id: 3 - body: - my_field1: "jumparound" - my_field2: "emu" - headers: - Content-Type: "application/vnd.elasticsearch+json;compatible-with=7" - Accept: "application/vnd.elasticsearch+json;compatible-with=7" - allowed_warnings_regex: - - "\\[types removal\\].*" - - do: - index: - index: "test" - id: 4 - body: - my_field1: "dog" - my_field2: "brown fox jump lazy" - headers: - Content-Type: "application/vnd.elasticsearch+json;compatible-with=7" - Accept: "application/vnd.elasticsearch+json;compatible-with=7" - allowed_warnings_regex: - - "\\[types removal\\].*" - do: indices.refresh: {} headers: @@ -80,7 +44,7 @@ setup: --- multi_match multiple fields with cutoff_frequency throws exception: - do: - catch: "/cutoff_freqency is not supported. The [match] query can fskip block of documents efficiently if the total number of hits is not tracked/" + catch: "/cutoff_freqency is not supported. The \\[multi_match\\] query can skip block of documents efficiently if the total number of hits is not tracked/" search: rest_total_hits_as_int: true index: "test" @@ -98,3 +62,44 @@ multi_match multiple fields with cutoff_frequency throws exception: Accept: "application/vnd.elasticsearch+json;compatible-with=7" allowed_warnings_regex: - "\\[types removal\\].*" + +--- +match with cutoff_frequency throws exception: + - do: + catch: "/cutoff_freqency is not supported. The \\[match\\] query can skip block of documents efficiently if the total number of hits is not tracked/" + search: + rest_total_hits_as_int: true + index: "test" + body: + query: + match: + my_field1: + query: "brown" + type: "bool_prefix" + cutoff_frequency: 0.001 + headers: + Content-Type: "application/vnd.elasticsearch+json;compatible-with=7" + Accept: "application/vnd.elasticsearch+json;compatible-with=7" + allowed_warnings_regex: + - "\\[types removal\\].*" + +--- +common querythrows exception: + - do: + catch: "/Common Terms Query usage is not supported. Use \\[match\\] query which can efficiently skip blocks of documents if the total number of hits is not tracked./" + search: + rest_total_hits_as_int: true + index: "test" + body: + query: + common: + my_field1: + query: "brown" + type: "bool_prefix" + cutoff_frequency: 0.001 + low_freq_operator: or + headers: + Content-Type: "application/vnd.elasticsearch+json;compatible-with=7" + Accept: "application/vnd.elasticsearch+json;compatible-with=7" + allowed_warnings_regex: + - "\\[types removal\\].*" diff --git a/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java index a3a77e810d4a8..d3cc5e6b32764 100644 --- a/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java @@ -21,8 +21,8 @@ public class CommonTermsQueryBuilder extends AbstractQueryBuilder { private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(CommonTermsQueryBuilder.class); - public static final String COMMON_TERMS_QUERY_DEPRECATION_MSG = "[match] query which can efficiently " + - "skip blocks of documents if the total number of hits is not tracked. Common Terms Query usage is not supported."; + public static final String COMMON_TERMS_QUERY_DEPRECATION_MSG = "Common Terms Query usage is not supported. " + + "Use [match] query which can efficiently skip blocks of documents if the total number of hits is not tracked."; public static ParseField NAME = new ParseField("common") .withAllDeprecated(COMMON_TERMS_QUERY_DEPRECATION_MSG) diff --git a/server/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java index a7ac3f63f43d6..7f7ca2c3d86b1 100644 --- a/server/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java @@ -43,8 +43,8 @@ public class MultiMatchQueryBuilder extends AbstractQueryBuilder { public static final String NAME = "multi_match"; - private static final String CUTOFF_FREQUENCY_DEPRECATION_MSG = "you can omit this option, " + - "the [multi_match] query can skip block of documents efficiently if the total number of hits is not tracked"; + private static final String CUTOFF_FREQUENCY_DEPRECATION_MSG = "cutoff_freqency is not supported." + + " The [multi_match] query can skip block of documents efficiently if the total number of hits is not tracked"; private static final ParseField CUTOFF_FREQUENCY_FIELD = new ParseField("cutoff_frequency") .withAllDeprecated(CUTOFF_FREQUENCY_DEPRECATION_MSG) diff --git a/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java b/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java index 9943864e830a0..b09db5f4ab730 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java @@ -674,7 +674,9 @@ public List> getQueries() { .filter(e -> RestApiVersion.minimumSupported().matches(e.restApiCompatibility)) .filter(e -> RestApiVersion.current().matches(e.restApiCompatibility)) .collect(toSet()), - hasSize(searchModule.getNamedXContents().size() - REST_COMPATIBLE_QUERIES.length -1 )); // -1 because of the registered in the test + // -1 because of the registered in the test + hasSize(searchModule.getNamedXContents().size() - REST_COMPATIBLE_QUERIES.length -1 )); + final List compatEntry = searchModule.getNamedXContents().stream() From 041fa9988fd5368842f3b90fc92d17d086d36e12 Mon Sep 17 00:00:00 2001 From: pgomulka Date: Tue, 3 Aug 2021 12:09:09 +0200 Subject: [PATCH 6/8] import --- .../test/java/org/elasticsearch/search/SearchModuleTests.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java b/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java index e3cae4ba7e373..55b7d212cb109 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java @@ -21,6 +21,7 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.index.query.AbstractQueryBuilder; +import org.elasticsearch.index.query.CommonTermsQueryBuilder; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryRewriteContext; import org.elasticsearch.index.query.SearchExecutionContext; From ad4be3f9b71a4d526494327d3f73a7e3d3cd3311 Mon Sep 17 00:00:00 2001 From: pgomulka Date: Tue, 3 Aug 2021 15:32:20 +0200 Subject: [PATCH 7/8] rename --- .../org/elasticsearch/index/query/CommonTermsQueryBuilder.java | 2 +- server/src/main/java/org/elasticsearch/search/SearchModule.java | 2 +- .../test/java/org/elasticsearch/search/SearchModuleTests.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java index d3cc5e6b32764..dee95fde5ebe9 100644 --- a/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java @@ -24,7 +24,7 @@ public class CommonTermsQueryBuilder extends AbstractQueryBuilder plugins) { ); if(RestApiVersion.minimumSupported() == RestApiVersion.V_7) { - registerQuery(new QuerySpec<>(CommonTermsQueryBuilder.NAME, + registerQuery(new QuerySpec<>(CommonTermsQueryBuilder.NAME_V7, (streamInput) -> new CommonTermsQueryBuilder(), CommonTermsQueryBuilder::fromXContent)); } diff --git a/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java b/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java index b42144c0f174d..2c2d4c6381876 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java @@ -396,7 +396,7 @@ public CheckedBiConsumer getReque private static final String[] DEPRECATED_QUERIES = new String[] {"field_masking_span", "geo_polygon"}; private static final String[] REST_COMPATIBLE_QUERIES = new String[] { TypeQueryV7Builder.NAME_V7.getPreferredName(), - CommonTermsQueryBuilder.NAME.getPreferredName() + CommonTermsQueryBuilder.NAME_V7.getPreferredName() }; /** From 33da26dec2b0cdd2a2e28bfe14a77a6d65466e76 Mon Sep 17 00:00:00 2001 From: pgomulka Date: Thu, 5 Aug 2021 11:40:16 +0200 Subject: [PATCH 8/8] unsupported opertaion exception --- .../org/elasticsearch/index/query/CommonTermsQueryBuilder.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java index dee95fde5ebe9..300a66b0e93c2 100644 --- a/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java @@ -30,6 +30,7 @@ public class CommonTermsQueryBuilder extends AbstractQueryBuilder