Introduce FilterRestHandler (#98861) (#98920)

RestHandler has a number of methods that affect the behaviour of request
processing. If the handler is wrapped (e.g. SecurityRestFilter or
DeprecationRestHandler) then these methods must be delegated to the
underlying handler.

This commit introduces a new abstract base class `FilterRestHandler`
that correctly delegates these methods so that wrappers (subclasses) do
not need to implement the behaviour on a case-by-case basis

Author: tvernum

server/src/main/java/org/elasticsearch/rest/DeprecationRestHandler.java

@@ -20,10 +20,10 @@
  * {@code DeprecationRestHandler} provides a proxy for any existing {@link RestHandler} so that usage of the handler can be
  * logged using the {@link DeprecationLogger}.
  */
-public class DeprecationRestHandler implements RestHandler {
+public class DeprecationRestHandler extends FilterRestHandler implements RestHandler {
 
     public static final String DEPRECATED_ROUTE_KEY = "deprecated_route";
-    private final RestHandler handler;
+
     private final String deprecationMessage;
     private final DeprecationLogger deprecationLogger;
     private final boolean compatibleVersionWarning;
@@ -55,7 +55,7 @@ public DeprecationRestHandler(
         DeprecationLogger deprecationLogger,
         boolean compatibleVersionWarning
     ) {
-        this.handler = Objects.requireNonNull(handler);
+        super(handler);
         this.deprecationMessage = requireValidHeader(deprecationMessage);
         this.deprecationLogger = Objects.requireNonNull(deprecationLogger);
         this.compatibleVersionWarning = compatibleVersionWarning;
@@ -92,17 +92,7 @@ public void handleRequest(RestRequest request, RestChannel channel, NodeClient c
             }
         }
 
-        handler.handleRequest(request, channel, client);
-    }
-
-    @Override
-    public RestHandler getConcreteRestHandler() {
-        return handler.getConcreteRestHandler();
-    }
-
-    @Override
-    public boolean supportsContentStream() {
-        return handler.supportsContentStream();
+        getDelegate().handleRequest(request, channel, client);
     }
 
     /**

server/src/main/java/org/elasticsearch/rest/FilterRestHandler.java

@@ -0,0 +1,59 @@
+/*
+ * 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.rest;
+
+import java.util.List;
+import java.util.Objects;
+
+public abstract class FilterRestHandler implements RestHandler {
+    private final RestHandler delegate;
+
+    protected FilterRestHandler(RestHandler delegate) {
+        this.delegate = Objects.requireNonNull(delegate);
+    }
+
+    protected RestHandler getDelegate() {
+        return delegate;
+    }
+
+    @Override
+    public RestHandler getConcreteRestHandler() {
+        return delegate.getConcreteRestHandler();
+    }
+
+    @Override
+    public List<RestHandler.Route> routes() {
+        return delegate.routes();
+    }
+
+    @Override
+    public boolean allowSystemIndexAccessByDefault() {
+        return delegate.allowSystemIndexAccessByDefault();
+    }
+
+    @Override
+    public boolean canTripCircuitBreaker() {
+        return delegate.canTripCircuitBreaker();
+    }
+
+    @Override
+    public boolean allowsUnsafeBuffers() {
+        return delegate.allowsUnsafeBuffers();
+    }
+
+    @Override
+    public boolean supportsContentStream() {
+        return delegate.supportsContentStream();
+    }
+
+    @Override
+    public boolean mediaTypesValid(RestRequest request) {
+        return delegate.mediaTypesValid(request);
+    }
+}

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/SecurityRestFilter.java

@@ -13,6 +13,7 @@
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.client.internal.node.NodeClient;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
+import org.elasticsearch.rest.FilterRestHandler;
 import org.elasticsearch.rest.RestChannel;
 import org.elasticsearch.rest.RestHandler;
 import org.elasticsearch.rest.RestRequest;
@@ -24,15 +25,12 @@
 import org.elasticsearch.xpack.security.authz.restriction.WorkflowService;
 import org.elasticsearch.xpack.security.operator.OperatorPrivileges;
 
-import java.util.List;
-
 import static org.elasticsearch.core.Strings.format;
 
-public class SecurityRestFilter implements RestHandler {
+public class SecurityRestFilter extends FilterRestHandler implements RestHandler {
 
     private static final Logger logger = LogManager.getLogger(SecurityRestFilter.class);
 
-    private final RestHandler restHandler;
     private final SecondaryAuthenticator secondaryAuthenticator;
     private final AuditTrailService auditTrailService;
     private final boolean enabled;
@@ -49,27 +47,18 @@ public SecurityRestFilter(
         RestHandler restHandler,
         OperatorPrivileges.OperatorPrivilegesService operatorPrivilegesService
     ) {
+        super(restHandler);
         this.enabled = enabled;
         this.threadContext = threadContext;
         this.secondaryAuthenticator = secondaryAuthenticator;
         this.auditTrailService = auditTrailService;
         this.workflowService = workflowService;
-        this.restHandler = restHandler;
         // can be null if security is not enabled
         this.operatorPrivilegesService = operatorPrivilegesService == null
             ? OperatorPrivileges.NOOP_OPERATOR_PRIVILEGES_SERVICE
             : operatorPrivilegesService;
     }
 
-    @Override
-    public boolean allowSystemIndexAccessByDefault() {
-        return restHandler.allowSystemIndexAccessByDefault();
-    }
-
-    public RestHandler getConcreteRestHandler() {
-        return restHandler.getConcreteRestHandler();
-    }
-
     @Override
     public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception {
         // requests with the OPTIONS method should be handled elsewhere, and not by calling {@code RestHandler#handleRequest}
@@ -94,17 +83,17 @@ public void handleRequest(RestRequest request, RestChannel channel, NodeClient c
             if (secondaryAuthentication != null) {
                 logger.trace("Found secondary authentication {} in REST request [{}]", secondaryAuthentication, request.uri());
             }
-            workflowService.resolveWorkflowAndStoreInThreadContext(restHandler, threadContext);
+            workflowService.resolveWorkflowAndStoreInThreadContext(getConcreteRestHandler(), threadContext);
             doHandleRequest(request, channel, client);
         }, e -> handleException(request, channel, e)));
     }
 
     private void doHandleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception {
         threadContext.sanitizeHeaders();
         // operator privileges can short circuit to return a non-successful response
-        if (operatorPrivilegesService.checkRest(restHandler, request, channel, threadContext)) {
+        if (operatorPrivilegesService.checkRest(getConcreteRestHandler(), request, channel, threadContext)) {
             try {
-                restHandler.handleRequest(request, channel, client);
+                getDelegate().handleRequest(request, channel, client);
             } catch (Exception e) {
                 logger.debug(() -> format("Request handling failed for REST request [%s]", request.uri()), e);
                 throw e;
@@ -123,40 +112,16 @@ protected void handleException(RestRequest request, RestChannel channel, Excepti
         }
     }
 
-    @Override
-    public boolean canTripCircuitBreaker() {
-        return restHandler.canTripCircuitBreaker();
-    }
-
-    @Override
-    public boolean supportsContentStream() {
-        return restHandler.supportsContentStream();
-    }
-
-    @Override
-    public boolean allowsUnsafeBuffers() {
-        return restHandler.allowsUnsafeBuffers();
-    }
-
-    @Override
-    public List<Route> routes() {
-        return restHandler.routes();
-    }
-
     // for testing
     OperatorPrivileges.OperatorPrivilegesService getOperatorPrivilegesService() {
         return operatorPrivilegesService;
     }
 
     private RestRequest maybeWrapRestRequest(RestRequest restRequest) {
-        if (restHandler instanceof RestRequestFilter) {
-            return ((RestRequestFilter) restHandler).getFilteredRequest(restRequest);
+        if (getConcreteRestHandler() instanceof RestRequestFilter rrf) {
+            return rrf.getFilteredRequest(restRequest);
         }
         return restRequest;
     }
 
-    @Override
-    public boolean mediaTypesValid(RestRequest request) {
-        return restHandler.mediaTypesValid(request);
-    }
 }