Browse Source

[Rest Api Compatibility] Make query registration easier (#75722)

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
Przemyslaw Gomulka 4 years ago
parent
commit
c8c5d22571

+ 52 - 58
libs/x-content/src/main/java/org/elasticsearch/common/xcontent/NamedXContentRegistry.java

@@ -12,12 +12,11 @@ import org.elasticsearch.core.CheckedFunction;
 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,82 @@ public class NamedXContentRegistry {
         /** A name for the entry which is unique within the {@link #categoryClass}. */
         public final ParseField name;
 
+        public final Function<RestApiVersion, Boolean> restApiCompatibility;
+
         /** A parser capability of parser the entry's class. */
         private final ContextParser<Object, ?> parser;
 
-        /** Creates a new entry which can be stored by the registry. */
+        /**
+         * Creates a new entry which can be stored by the registry.
+         */
         public <T> Entry(Class<T> categoryClass, ParseField name, CheckedFunction<XContentParser, ? extends T, IOException> 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), name.getForRestApiVersion());
+        }
+
+        public <T> Entry(Class<T> categoryClass, ParseField name, CheckedFunction<XContentParser, ? extends T, IOException> parser,
+                         Function<RestApiVersion, Boolean> 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 <T> Entry(Class<T> categoryClass, ParseField name, ContextParser<Object, ? extends T> parser) {
+            this(categoryClass, name, parser, name.getForRestApiVersion());
+        }
+
+        public <T> Entry(Class<T> categoryClass, ParseField name, ContextParser<Object, ? extends T> parser,
+                         Function<RestApiVersion, Boolean> restApiCompatibility) {
             this.categoryClass = Objects.requireNonNull(categoryClass);
             this.name = Objects.requireNonNull(name);
             this.parser = Objects.requireNonNull(parser);
+            this.restApiCompatibility = restApiCompatibility;
         }
     }
 
-    private final Map<Class<?>, Map<String, Entry>> registry;
-    private final Map<Class<?>, Map<String, Entry>> compatibleRegistry;
+    private final Map<RestApiVersion,Map<Class<?>, Map<String, Entry>>> registry;
 
-    public NamedXContentRegistry(List<Entry> entries){
-        this(entries, Collections.emptyList());
-    }
 
-    public NamedXContentRegistry(List<Entry> entries, List<Entry> compatibleEntries) {
-        this.registry = unmodifiableMap(getRegistry(entries));
-        this.compatibleRegistry = unmodifiableMap(getCompatibleRegistry(compatibleEntries));
+    public NamedXContentRegistry(List<Entry> entries) {
+        this.registry = unmodifiableMap(createRegistry(entries));
     }
 
-    private Map<Class<?>, Map<String, Entry>> getCompatibleRegistry(List<Entry> compatibleEntries) {
-        Map<Class<?>, Map<String, Entry>> compatibleRegistry = new HashMap<>(registry);
-        List<Entry> unseenEntries = new ArrayList<>();
-        compatibleEntries.forEach(entry -> {
-                Map<String, Entry> parsers = compatibleRegistry.get(entry.categoryClass);
-                if (parsers == null) {
-                    unseenEntries.add(entry);
-                } else {
-                    Map<String, Entry> 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<Class<?>, Map<String, Entry>> getRegistry(List<Entry> entries){
+    private  Map<RestApiVersion,Map<Class<?>, Map<String, Entry>>> createRegistry(List<Entry> entries){
         if (entries.isEmpty()) {
             return emptyMap();
         }
-        entries = new ArrayList<>(entries);
-        entries.sort((e1, e2) -> e1.categoryClass.getName().compareTo(e2.categoryClass.getName()));
 
-        Map<Class<?>, Map<String, Entry>> registry = new HashMap<>();
-        Map<String, Entry> parsers = null;
-        Class<?> currentCategory = null;
+        Map<RestApiVersion,Map<Class<?>, Map<String, Entry>>> 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<RestApiVersion, Map<Class<?>, Map<String, Entry>>> registry,
+                                 Entry entry,
+                                 String name,
+                                 RestApiVersion restApiVersion) {
+        final Map<Class<?>, Map<String, Entry>> classRegistry =
+            registry.computeIfAbsent(restApiVersion, (v) -> new HashMap<>());
+        final Map<String, Entry> 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 +126,14 @@ public class NamedXContentRegistry {
      * @throws NamedObjectNotFoundException if the categoryClass or name is not registered
      */
     public <T, C> T parseNamedObject(Class<T> categoryClass, String name, XContentParser parser, C context) throws IOException {
+        Entry entry = lookupParser(categoryClass, name, parser);
+        return categoryClass.cast(entry.parser.parse(parser, context));
+    }
 
-        Map<String, Entry> parsers = parser.getRestApiVersion() == RestApiVersion.minimumSupported() ?
-            compatibleRegistry.get(categoryClass) : registry.get(categoryClass);
+    //scope for testing
+    public <T> Entry lookupParser(Class<T> categoryClass, String name, XContentParser parser) {
+        Map<String, Entry> 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.
@@ -157,7 +151,7 @@ public class NamedXContentRegistry {
             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;
     }
 
 }

+ 4 - 7
server/src/main/java/org/elasticsearch/node/Node.java

@@ -258,7 +258,9 @@ public class Node implements Closeable {
     private final Collection<LifecycleComponent> 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);
@@ -420,8 +422,7 @@ public class Node implements Closeable {
                 pluginsService.filterPlugins(Plugin.class).stream()
                     .flatMap(p -> p.getNamedXContent().stream()),
                 ClusterModule.getNamedXWriteables().stream())
-                .flatMap(Function.identity()).collect(toList()),
-                getCompatibleNamedXContents()
+                .flatMap(Function.identity()).collect(toList())
             );
             final Map<String, SystemIndices.Feature> featuresMap = pluginsService
                 .filterPlugins(SystemIndexPlugin.class)
@@ -736,6 +737,7 @@ public class Node implements Closeable {
                 transportService.getRemoteClusterService(),
                 namedWriteableRegistry);
             this.namedWriteableRegistry = namedWriteableRegistry;
+            this.namedXContentRegistry = xContentRegistry;
 
             logger.debug("initializing HTTP handlers ...");
             actionModule.initRestHandlers(() -> clusterService.state().nodes());
@@ -751,11 +753,6 @@ public class Node implements Closeable {
         }
     }
 
-    // package scope for testing
-    List<NamedXContentRegistry.Entry> getCompatibleNamedXContents() {
-        return pluginsService.filterPlugins(Plugin.class).stream()
-            .flatMap(p -> p.getNamedXContentForCompatibility().stream()).collect(toList());
-    }
 
     protected TransportService newTransportService(Settings settings, Transport transport, ThreadPool threadPool,
                                                    TransportInterceptor interceptor,

+ 0 - 9
server/src/main/java/org/elasticsearch/plugins/Plugin.java

@@ -112,15 +112,6 @@ public abstract class Plugin implements Closeable {
         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<NamedXContentRegistry.Entry> 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.

+ 1 - 1
server/src/main/java/org/elasticsearch/search/SearchModule.java

@@ -881,7 +881,7 @@ public class SearchModule {
     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<QueryBuilder> reader) {

+ 33 - 23
server/src/test/java/org/elasticsearch/common/xcontent/CompatibleNamedXContentRegistryTests.java

@@ -16,10 +16,10 @@ import org.elasticsearch.test.ESTestCase;
 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;
+import java.util.function.Function;
 
 import static org.hamcrest.core.IsEqual.equalTo;
 
@@ -31,18 +31,18 @@ public class CompatibleNamedXContentRegistryTests extends ESTestCase {
     static class ParentObject {
         private static final ConstructingObjectParser<ParentObject, String> 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;
         }
@@ -53,10 +53,13 @@ public class CompatibleNamedXContentRegistryTests extends ESTestCase {
         }
     }
 
-    static class SubObject {
-        private static final ConstructingObjectParser<SubObject, String> PARSER = new ConstructingObjectParser<>(
+    static class NewSubObject {
+        public static final Function<RestApiVersion, Boolean> REST_API_VERSION = RestApiVersion.onOrAfter(RestApiVersion.current());
+        private static final ConstructingObjectParser<NewSubObject, String> 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()));
 
         static {
             PARSER.declareString(ConstructingObjectParser.constructorArg(), new ParseField("new_field"));
@@ -64,37 +67,43 @@ public class CompatibleNamedXContentRegistryTests extends ESTestCase {
 
         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<SubObject, String> PARSER = new ConstructingObjectParser<>(
-            "parser2", false,
-            a -> new SubObject((String) a[0]));
+    static class OldSubObject {
+        public static final Function<RestApiVersion, Boolean> REST_API_VERSION = RestApiVersion.equalTo(RestApiVersion.minimumSupported());
 
+        private static final ConstructingObjectParser<NewSubObject, String> PARSER = new ConstructingObjectParser<>(
+            "parser2", false,
+            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(
-            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(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();
@@ -120,13 +129,14 @@ public class CompatibleNamedXContentRegistryTests extends ESTestCase {
             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(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();
@@ -141,7 +151,7 @@ public class CompatibleNamedXContentRegistryTests extends ESTestCase {
                 String.valueOf(RestApiVersion.minimumSupported().major)));
         List<String> 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))

+ 58 - 56
server/src/test/java/org/elasticsearch/node/NodeTests.java

@@ -12,14 +12,20 @@ import org.apache.lucene.util.SetOnce;
 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.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;
 import org.elasticsearch.index.engine.Engine.Searcher;
@@ -29,17 +35,19 @@ import org.elasticsearch.indices.breaker.BreakerSettings;
 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;
@@ -49,12 +57,12 @@ import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF
 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;
 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 {
@@ -335,72 +343,66 @@ public class NodeTests extends ESTestCase {
         }
     }
 
-
-    interface MockRestApiVersion {
-        RestApiVersion minimumRestCompatibilityVersion();
-    }
-
-    static MockRestApiVersion MockCompatibleVersion = Mockito.mock(MockRestApiVersion.class);
-
-    static NamedXContentRegistry.Entry v7CompatibleEntries = new NamedXContentRegistry.Entry(Integer.class,
-        new ParseField("name"), Mockito.mock(ContextParser.class));
-    static NamedXContentRegistry.Entry v8CompatibleEntries = new NamedXContentRegistry.Entry(Integer.class,
-        new ParseField("name2"), Mockito.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<NamedXContentRegistry.Entry> getNamedXContentForCompatibility() {
-            // real plugin will use CompatibleVersion.minimumRestCompatibilityVersion()
-            if (/*CompatibleVersion.minimumRestCompatibilityVersion()*/
-                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()*/
-                MockCompatibleVersion.minimumRestCompatibilityVersion().equals(RestApiVersion.V_8)) {
-                return List.of(v8CompatibleEntries);
+        public List<NamedXContentRegistry.Entry> getNamedXContent() {
+            return List.of(compatibleEntries);
+        }
+    }
 
-            }
-            return super.getNamedXContentForCompatibility();
+    public static class TestRestCompatibility2 extends Plugin {
+        @Override
+        public List<NamedXContentRegistry.Entry> 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<Class<? extends Plugin>> 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<Class<? extends Plugin>> 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<NamedXContentRegistry.Entry> compatibleNamedXContents = node.getCompatibleNamedXContents();
-                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<Class<? extends Plugin>> plugins = basePlugins();
-            plugins.add(TestRestCompatibility1.class);
-
-            try (Node node = new MockNode(settings.build(), plugins)) {
-                List<NamedXContentRegistry.Entry> compatibleNamedXContents = node.getCompatibleNamedXContents();
-                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<String> 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();
+    }
+
 
 }

+ 73 - 0
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.io.stream.StreamOutput;
 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;
@@ -607,4 +611,73 @@ public class SearchModuleTests extends ESTestCase {
             return "test";
         }
     }
+
+    static class CompatQueryBuilder extends AbstractQueryBuilder<CompatQueryBuilder> {
+        public static final String NAME = "compat_name";
+        public static final ParseField NAME_OLD = 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<SearchPlugin.QuerySpec<?>> getQueries() {
+                return singletonList(new QuerySpec<>(CompatQueryBuilder.NAME_OLD,
+                    (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_OLD.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<NamedXContentRegistry.Entry> compatEntry = searchModule.getNamedXContents().stream()
+            .filter(e -> e.categoryClass.equals(QueryBuilder.class) &&
+                e.name.match(CompatQueryBuilder.NAME_OLD.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));
+    }
 }