فهرست منبع

Add MappedActionFilters for efficient action filters (#99833)

ActionFilters are applied linearly, and for all actions. Yet many times
an action filterly wants to be applied to a single action, and not
invoked when other actions are processed. This commit adds a new special
kind of action filter, a MappedActionFilter, which applies to a single
action name. The MappedActionFilters are collected just as normal action
filters during startup, but are then held in a hash map for quick access
when processing action filters at runtime.

Some implementation notes: MappedActionFilters all run at the same
action filter order, 0. Relative to each other, if more than one
MappedActionFilter exists for the same action name, they are run in
definition order (which should be considered effectively arbitrary).

closes #99784
Ryan Ernst 2 سال پیش
والد
کامیت
1738ffc87f

+ 18 - 4
server/src/main/java/org/elasticsearch/action/ActionModule.java

@@ -247,9 +247,12 @@ import org.elasticsearch.action.search.TransportOpenPointInTimeAction;
 import org.elasticsearch.action.search.TransportSearchAction;
 import org.elasticsearch.action.search.TransportSearchScrollAction;
 import org.elasticsearch.action.search.TransportSearchShardsAction;
+import org.elasticsearch.action.support.ActionFilter;
 import org.elasticsearch.action.support.ActionFilters;
 import org.elasticsearch.action.support.AutoCreateIndex;
 import org.elasticsearch.action.support.DestructiveOperations;
+import org.elasticsearch.action.support.MappedActionFilter;
+import org.elasticsearch.action.support.MappedActionFilters;
 import org.elasticsearch.action.support.TransportAction;
 import org.elasticsearch.action.synonyms.DeleteSynonymRuleAction;
 import org.elasticsearch.action.synonyms.DeleteSynonymsAction;
@@ -463,7 +466,6 @@ import org.elasticsearch.usage.UsageService;
 
 import java.time.Instant;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -844,9 +846,21 @@ public class ActionModule extends AbstractModule {
     }
 
     private static ActionFilters setupActionFilters(List<ActionPlugin> actionPlugins) {
-        return new ActionFilters(
-            Collections.unmodifiableSet(actionPlugins.stream().flatMap(p -> p.getActionFilters().stream()).collect(Collectors.toSet()))
-        );
+        List<ActionFilter> finalFilters = new ArrayList<>();
+        List<MappedActionFilter> mappedFilters = new ArrayList<>();
+        for (var plugin : actionPlugins) {
+            for (var filter : plugin.getActionFilters()) {
+                if (filter instanceof MappedActionFilter mappedFilter) {
+                    mappedFilters.add(mappedFilter);
+                } else {
+                    finalFilters.add(filter);
+                }
+            }
+        }
+        if (mappedFilters.isEmpty() == false) {
+            finalFilters.add(new MappedActionFilters(mappedFilters));
+        }
+        return new ActionFilters(Set.copyOf(finalFilters));
     }
 
     public void initRestHandlers(Supplier<DiscoveryNodes> nodesInCluster) {

+ 13 - 0
server/src/main/java/org/elasticsearch/action/support/MappedActionFilter.java

@@ -0,0 +1,13 @@
+/*
+ * 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.action.support;
+
+public interface MappedActionFilter extends ActionFilter {
+    String actionName();
+}

+ 74 - 0
server/src/main/java/org/elasticsearch/action/support/MappedActionFilters.java

@@ -0,0 +1,74 @@
+/*
+ * 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.action.support;
+
+import org.elasticsearch.action.ActionListener;
+import org.elasticsearch.action.ActionRequest;
+import org.elasticsearch.action.ActionResponse;
+import org.elasticsearch.tasks.Task;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+public class MappedActionFilters implements ActionFilter {
+
+    private final Map<String, List<MappedActionFilter>> filtersByAction;
+
+    public MappedActionFilters(List<MappedActionFilter> mappedFilters) {
+        Map<String, List<MappedActionFilter>> map = new HashMap<>();
+        for (var filter : mappedFilters) {
+            map.computeIfAbsent(filter.actionName(), k -> new ArrayList<>()).add(filter);
+        }
+        map.replaceAll((k, l) -> List.copyOf(l));
+        this.filtersByAction = Map.copyOf(map);
+    }
+
+    @Override
+    public int order() {
+        return 0;
+    }
+
+    @Override
+    public <Request extends ActionRequest, Response extends ActionResponse> void apply(
+        Task task,
+        String action,
+        Request request,
+        ActionListener<Response> listener,
+        ActionFilterChain<Request, Response> outerChain
+    ) {
+        var chain = new MappedFilterChain<>(this.filtersByAction.getOrDefault(action, List.of()), outerChain);
+        chain.proceed(task, action, request, listener);
+    }
+
+    private static class MappedFilterChain<Request extends ActionRequest, Response extends ActionResponse>
+        implements
+            ActionFilterChain<Request, Response> {
+
+        final List<MappedActionFilter> filters;
+        final ActionFilterChain<Request, Response> outerChain;
+        int index = 0;
+
+        MappedFilterChain(List<MappedActionFilter> filters, ActionFilterChain<Request, Response> outerChain) {
+            this.filters = filters;
+            this.outerChain = outerChain;
+        }
+
+        @Override
+        public void proceed(Task task, String action, Request request, ActionListener<Response> listener) {
+            if (index < filters.size()) {
+                var filter = filters.get(index++);
+                filter.apply(task, action, request, listener, this);
+            } else {
+                outerChain.proceed(task, action, request, listener);
+            }
+        }
+    }
+}

+ 196 - 0
server/src/test/java/org/elasticsearch/action/support/MappedActionFiltersTests.java

@@ -0,0 +1,196 @@
+/*
+ * 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.action.support;
+
+import org.elasticsearch.action.ActionListener;
+import org.elasticsearch.action.ActionRequest;
+import org.elasticsearch.action.ActionRequestValidationException;
+import org.elasticsearch.action.ActionResponse;
+import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.tasks.Task;
+import org.elasticsearch.test.ESTestCase;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import static org.hamcrest.Matchers.is;
+
+public class MappedActionFiltersTests extends ESTestCase {
+
+    static class DummyRequest extends ActionRequest {
+        @Override
+        public ActionRequestValidationException validate() {
+            return null;
+        }
+    }
+
+    static class DummyResponse extends ActionResponse {
+        @Override
+        public void writeTo(StreamOutput out) throws IOException {}
+    }
+
+    public void testNoMappedFilters() {
+        var actionFilter = new MappedActionFilters(List.of());
+        AtomicBoolean proceedCalled = new AtomicBoolean();
+        var chain = new ActionFilterChain<DummyRequest, DummyResponse>() {
+            @Override
+            public void proceed(Task task, String action, DummyRequest request, ActionListener<DummyResponse> listener) {
+                proceedCalled.set(true);
+            }
+        };
+        actionFilter.apply(null, "dnm", null, null, chain);
+        assertThat(proceedCalled.get(), is(true));
+    }
+
+    public void testSingleMappedFilters() {
+        AtomicBoolean applyCalled = new AtomicBoolean();
+        AtomicBoolean proceedCalled = new AtomicBoolean();
+
+        MappedActionFilter filter = new MappedActionFilter() {
+            @Override
+            public String actionName() {
+                return "dummyAction";
+            }
+
+            @Override
+            public int order() {
+                return 0;
+            }
+
+            @Override
+            public <Request extends ActionRequest, Response extends ActionResponse> void apply(
+                Task task,
+                String action,
+                Request request,
+                ActionListener<Response> listener,
+                ActionFilterChain<Request, Response> chain
+            ) {
+                applyCalled.set(true);
+                chain.proceed(task, action, request, listener);
+            }
+        };
+        var actionFilter = new MappedActionFilters(List.of(filter));
+
+        var chain = new ActionFilterChain<DummyRequest, DummyResponse>() {
+            @Override
+            public void proceed(Task task, String action, DummyRequest request, ActionListener<DummyResponse> listener) {
+                assertThat("mapped filter should be called first", applyCalled.get(), is(true));
+                proceedCalled.set(true);
+            }
+        };
+        actionFilter.apply(null, "dummyAction", null, null, chain);
+        assertThat(proceedCalled.get(), is(true));
+    }
+
+    public void testMultipleMappedFilters() {
+        AtomicBoolean apply1Called = new AtomicBoolean();
+        AtomicBoolean apply2Called = new AtomicBoolean();
+        AtomicBoolean proceedCalled = new AtomicBoolean();
+
+        MappedActionFilter filter1 = new MappedActionFilter() {
+            @Override
+            public String actionName() {
+                return "dummyAction";
+            }
+
+            @Override
+            public int order() {
+                return 0;
+            }
+
+            @Override
+            public <Request extends ActionRequest, Response extends ActionResponse> void apply(
+                Task task,
+                String action,
+                Request request,
+                ActionListener<Response> listener,
+                ActionFilterChain<Request, Response> chain
+            ) {
+                apply1Called.set(true);
+                chain.proceed(task, action, request, listener);
+            }
+        };
+        MappedActionFilter filter2 = new MappedActionFilter() {
+            @Override
+            public String actionName() {
+                return "dummyAction";
+            }
+
+            @Override
+            public int order() {
+                return 0;
+            }
+
+            @Override
+            public <Request extends ActionRequest, Response extends ActionResponse> void apply(
+                Task task,
+                String action,
+                Request request,
+                ActionListener<Response> listener,
+                ActionFilterChain<Request, Response> chain
+            ) {
+                assertThat("filter1 should be called first", apply1Called.get(), is(true));
+                apply2Called.set(true);
+                chain.proceed(task, action, request, listener);
+            }
+        };
+        var actionFilter = new MappedActionFilters(List.of(filter1, filter2));
+
+        var chain = new ActionFilterChain<DummyRequest, DummyResponse>() {
+            @Override
+            public void proceed(Task task, String action, DummyRequest request, ActionListener<DummyResponse> listener) {
+                assertThat("filter2 should be called before outer proceed", apply2Called.get(), is(true));
+                proceedCalled.set(true);
+            }
+        };
+        actionFilter.apply(null, "dummyAction", null, null, chain);
+        assertThat(proceedCalled.get(), is(true));
+    }
+
+    public void testSkipOtherAction() {
+        AtomicBoolean applyCalled = new AtomicBoolean();
+        AtomicBoolean proceedCalled = new AtomicBoolean();
+
+        MappedActionFilter filter = new MappedActionFilter() {
+            @Override
+            public String actionName() {
+                return "dummyAction";
+            }
+
+            @Override
+            public int order() {
+                return 0;
+            }
+
+            @Override
+            public <Request extends ActionRequest, Response extends ActionResponse> void apply(
+                Task task,
+                String action,
+                Request request,
+                ActionListener<Response> listener,
+                ActionFilterChain<Request, Response> chain
+            ) {
+                applyCalled.set(true);
+                chain.proceed(task, action, request, listener);
+            }
+        };
+        var actionFilter = new MappedActionFilters(List.of(filter));
+
+        var chain = new ActionFilterChain<DummyRequest, DummyResponse>() {
+            @Override
+            public void proceed(Task task, String action, DummyRequest request, ActionListener<DummyResponse> listener) {
+                proceedCalled.set(true);
+            }
+        };
+        actionFilter.apply(null, "differentAction", null, null, chain);
+        assertThat(applyCalled.get(), is(false));
+        assertThat(proceedCalled.get(), is(true));
+    }
+}