This is an automated email from the ASF dual-hosted git repository.
Fly-Style pushed a commit to branch 39.0.0
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/39.0.0 by this push:
new be11dde6842 fix: Restore the shutdownAllTasks Overlord API (#20824)
(#20834)
be11dde6842 is described below
commit be11dde68420757cb12b7d1bad893e99ef937554
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Wed Nov 4 20:37:26 2026 +0200
fix: Restore the shutdownAllTasks Overlord API (#20824) (#20834)
* fix: Restore the shutdownAllTasks Overlord API
POST /druid/indexer/v1/datasources/{datasource}/shutdownAllTasks has
returned 404 since 32.0.0. OverlordDataSourcesResource, added in #17545,
owns the /druid/indexer/v1/datasources path, and Jersey doesn't fall back
to OverlordResource for requests under it, so the endpoint could no
longer be reached.
Move the endpoint to OverlordDataSourcesResource, keeping the same URL
and behavior, and add an embedded test that calls it over HTTP.
Fixes #18419
* test: Check that no Jersey endpoint is shadowed by another resource
Jersey routes each request to a single root resource class and never falls
back to another one, so an endpoint is unreachable when its path falls under
a more specific @Path of another resource class. That is how
shutdownAllTasks
broke in 32.0.0.
MainTest now checks every server type for such endpoints, using Jersey's own
resource model and path matching. Without the fix, it reports exactly the
shutdownAllTasks endpoint of OverlordResource.
---------
(cherry picked from commit a88e765203ce81b2e3caa332ce33e43ccb9ba70d)
Co-authored-by: Nagarjunanm <[email protected]>
Co-authored-by: n0n02ip <[email protected]>
---
.../embedded/server/OverlordClientTest.java | 28 ++++
.../overlord/http/OverlordDataSourcesResource.java | 38 ++++-
.../indexing/overlord/http/OverlordResource.java | 23 ---
.../http/OverlordDataSourcesResourceTest.java | 61 ++++++-
.../overlord/http/OverlordResourceTest.java | 53 ------
services/pom.xml | 4 +
.../apache/druid/cli/JerseyShadowedEndpoints.java | 186 +++++++++++++++++++++
.../druid/cli/JerseyShadowedEndpointsTest.java | 163 ++++++++++++++++++
.../test/java/org/apache/druid/cli/MainTest.java | 36 +++-
9 files changed, 510 insertions(+), 82 deletions(-)
diff --git
a/embedded-tests/src/test/java/org/apache/druid/testing/embedded/server/OverlordClientTest.java
b/embedded-tests/src/test/java/org/apache/druid/testing/embedded/server/OverlordClientTest.java
index eae3a492f6a..1bba0185600 100644
---
a/embedded-tests/src/test/java/org/apache/druid/testing/embedded/server/OverlordClientTest.java
+++
b/embedded-tests/src/test/java/org/apache/druid/testing/embedded/server/OverlordClientTest.java
@@ -19,9 +19,11 @@
package org.apache.druid.testing.embedded.server;
+import com.fasterxml.jackson.core.type.TypeReference;
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
import com.google.common.util.concurrent.ListenableFuture;
+import io.netty.handler.codec.http.HttpMethod;
import org.apache.druid.client.indexing.IndexingTotalWorkerCapacityInfo;
import org.apache.druid.client.indexing.IndexingWorkerInfo;
import org.apache.druid.common.utils.IdUtils;
@@ -36,6 +38,7 @@ import org.apache.druid.java.util.common.Intervals;
import org.apache.druid.java.util.common.StringUtils;
import org.apache.druid.java.util.common.parsers.CloseableIterator;
import org.apache.druid.rpc.HttpResponseException;
+import org.apache.druid.rpc.RequestBuilder;
import org.apache.druid.rpc.indexing.OverlordClient;
import org.apache.druid.rpc.indexing.SegmentUpdateResponse;
import org.apache.druid.segment.TestDataSource;
@@ -134,6 +137,31 @@ public class OverlordClientTest extends
EmbeddedClusterTestBase
);
}
+ @Test
+ public void test_shutdownAllTasks_forDatasourceWithActiveTask()
+ {
+ // Submit a task that would run for much longer than this test
+ final String taskId = IdUtils.newTaskId("sim_test_noop", dataSource, null);
+ cluster.callApi().submitTask(new NoopTask(taskId, null, dataSource,
300_000L, 0L, null));
+
+ // Send a real HTTP request, since a routing conflict made this API return
404 (#18419)
+ final Map<String, String> response =
cluster.callApi().serviceClient().onLeaderOverlord(
+ mapper -> new RequestBuilder(
+ HttpMethod.POST,
+ StringUtils.format(
+ "/druid/indexer/v1/datasources/%s/shutdownAllTasks",
+ StringUtils.urlEncode(dataSource)
+ )
+ ),
+ new TypeReference<>() {}
+ );
+ Assertions.assertEquals(Map.of("dataSource", dataSource), response);
+
+ final TaskStatus taskStatus =
cluster.callApi().waitForTaskToFinish(taskId, overlord.latchableEmitter());
+ Assertions.assertEquals(TaskState.FAILED, taskStatus.getStatusCode());
+ Assertions.assertEquals("Shutdown request from user",
taskStatus.getErrorMsg());
+ }
+
@Test
public void test_taskStatuses_returnsEmpty_forRunningTasks()
{
diff --git
a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordDataSourcesResource.java
b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordDataSourcesResource.java
index 3e9254a9c41..69f874bf8d4 100644
---
a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordDataSourcesResource.java
+++
b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordDataSourcesResource.java
@@ -19,6 +19,7 @@
package org.apache.druid.indexing.overlord.http;
+import com.google.common.base.Optional;
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableMap;
import com.google.inject.Inject;
@@ -27,8 +28,11 @@ import org.apache.druid.audit.AuditEntry;
import org.apache.druid.audit.AuditManager;
import org.apache.druid.common.utils.IdUtils;
import org.apache.druid.error.DruidException;
+import org.apache.druid.indexer.TaskInfo;
import org.apache.druid.indexing.overlord.IndexerMetadataStorageCoordinator;
import org.apache.druid.indexing.overlord.TaskMaster;
+import org.apache.druid.indexing.overlord.TaskQueryTool;
+import org.apache.druid.indexing.overlord.TaskQueue;
import org.apache.druid.java.util.common.StringUtils;
import org.apache.druid.java.util.common.logger.Logger;
import org.apache.druid.rpc.indexing.SegmentUpdateResponse;
@@ -55,9 +59,13 @@ import java.util.Set;
import java.util.stream.Collectors;
/**
- * Datasource APIs exposed by the Overlord to update segments.
- * Some of these APIs are also exposed by the Coordinator, but they have been
+ * Datasource APIs exposed by the Overlord to update segments and shut down
tasks.
+ * Some of the segment APIs are also exposed by the Coordinator, but they have
been
* deprecated and the Overlord APIs must be used for all update operations.
+ * <p>
+ * All Overlord APIs under {@code /druid/indexer/v1/datasources} must be
defined in this class.
+ * Jersey routes every request under that path here and never falls back to
{@link OverlordResource},
+ * so a method defined there for such a path can't be reached.
*/
@Path("/druid/indexer/v1/datasources")
public class OverlordDataSourcesResource
@@ -66,16 +74,19 @@ public class OverlordDataSourcesResource
private final IndexerMetadataStorageCoordinator metadataStorageCoordinator;
private final TaskMaster taskMaster;
+ private final TaskQueryTool taskQueryTool;
private final AuditManager auditManager;
@Inject
public OverlordDataSourcesResource(
TaskMaster taskMaster,
+ TaskQueryTool taskQueryTool,
IndexerMetadataStorageCoordinator metadataStorageCoordinator,
AuditManager auditManager
)
{
this.taskMaster = taskMaster;
+ this.taskQueryTool = taskQueryTool;
this.auditManager = auditManager;
this.metadataStorageCoordinator = metadataStorageCoordinator;
}
@@ -256,6 +267,29 @@ public class OverlordDataSourcesResource
return performSegmentUpdate(dataSourceName, operation);
}
+ @POST
+ @Path("/{dataSourceName}/shutdownAllTasks")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ResourceFilters(DatasourceResourceFilter.class)
+ public Response shutdownTasksForDataSource(@PathParam("dataSourceName")
final String dataSourceName)
+ {
+ final Optional<TaskQueue> taskQueue = taskMaster.getTaskQueue();
+ if (!taskQueue.isPresent()) {
+ // Encourage client to try again soon, when we'll likely have a redirect
set up
+ return Response.status(Response.Status.SERVICE_UNAVAILABLE).build();
+ }
+
+ final List<TaskInfo> tasks =
taskQueryTool.getActiveTaskInfo(dataSourceName);
+ if (tasks.isEmpty()) {
+ return Response.status(Response.Status.NOT_FOUND).build();
+ }
+
+ for (final TaskInfo task : tasks) {
+ taskQueue.get().shutdown(task.getId(), "Shutdown request from user");
+ }
+ return Response.ok(ImmutableMap.of("dataSource", dataSourceName)).build();
+ }
+
private Response performSegmentUpdate(String dataSourceName,
SegmentUpdateOperation operation)
{
if (!taskMaster.isHalfOrFullLeader()) {
diff --git
a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordResource.java
b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordResource.java
index 6c2efddd746..a78849d0845 100644
---
a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordResource.java
+++
b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordResource.java
@@ -69,7 +69,6 @@ import org.apache.druid.metadata.LockFilterPolicy;
import org.apache.druid.server.http.HttpMediaType;
import org.apache.druid.server.http.ServletResourceUtils;
import org.apache.druid.server.http.security.ConfigResourceFilter;
-import org.apache.druid.server.http.security.DatasourceResourceFilter;
import org.apache.druid.server.http.security.StateResourceFilter;
import org.apache.druid.server.security.Action;
import org.apache.druid.server.security.AuthConfig;
@@ -397,28 +396,6 @@ public class OverlordResource
);
}
- @POST
- @Path("/datasources/{dataSource}/shutdownAllTasks")
- @Produces(MediaType.APPLICATION_JSON)
- @ResourceFilters(DatasourceResourceFilter.class)
- public Response shutdownTasksForDataSource(@PathParam("dataSource") final
String dataSource)
- {
- return asLeaderWith(
- taskMaster.getTaskQueue(),
- taskQueue -> {
- final List<TaskInfo> tasks =
taskQueryTool.getActiveTaskInfo(dataSource);
- if (tasks.isEmpty()) {
- return Response.status(Status.NOT_FOUND).build();
- } else {
- for (final TaskInfo task : tasks) {
- taskQueue.shutdown(task.getId(), "Shutdown request from user");
- }
- return Response.ok(ImmutableMap.of("dataSource",
dataSource)).build();
- }
- }
- );
- }
-
@POST
@Path("/taskStatus")
@Produces(MediaType.APPLICATION_JSON)
diff --git
a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/http/OverlordDataSourcesResourceTest.java
b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/http/OverlordDataSourcesResourceTest.java
index d83ae340a4d..6b0b96844a8 100644
---
a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/http/OverlordDataSourcesResourceTest.java
+++
b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/http/OverlordDataSourcesResourceTest.java
@@ -20,9 +20,16 @@
package org.apache.druid.indexing.overlord.http;
import org.apache.druid.audit.AuditManager;
+import org.apache.druid.indexer.TaskInfo;
+import org.apache.druid.indexer.TaskStatus;
+import org.apache.druid.indexing.common.task.NoopTask;
+import org.apache.druid.indexing.common.task.Task;
import org.apache.druid.indexing.overlord.Segments;
import org.apache.druid.indexing.overlord.TaskMaster;
+import org.apache.druid.indexing.overlord.TaskQueryTool;
+import org.apache.druid.indexing.overlord.TaskQueue;
import org.apache.druid.indexing.test.TestIndexerMetadataStorageCoordinator;
+import org.apache.druid.java.util.common.DateTimes;
import org.apache.druid.java.util.common.Intervals;
import org.apache.druid.java.util.common.granularity.Granularities;
import org.apache.druid.rpc.indexing.SegmentUpdateResponse;
@@ -42,6 +49,7 @@ import javax.ws.rs.core.Response;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
+import java.util.Map;
import java.util.Set;
public class OverlordDataSourcesResourceTest
@@ -54,6 +62,9 @@ public class OverlordDataSourcesResourceTest
.eachOfSizeInMb(500);
private TestIndexerMetadataStorageCoordinator storageCoordinator;
+ private TaskMaster taskMaster;
+ private TaskQueue taskQueue;
+ private TaskQueryTool taskQueryTool;
private OverlordDataSourcesResource dataSourcesResource;
@@ -62,13 +73,17 @@ public class OverlordDataSourcesResourceTest
{
AuditManager auditManager = EasyMock.createStrictMock(AuditManager.class);
storageCoordinator = new TestIndexerMetadataStorageCoordinator();
+ taskQueue = EasyMock.createStrictMock(TaskQueue.class);
+ taskQueryTool = EasyMock.createMock(TaskQueryTool.class);
- TaskMaster taskMaster = new TaskMaster(null, null);
+ taskMaster = new TaskMaster(null, null);
dataSourcesResource = new OverlordDataSourcesResource(
taskMaster,
+ taskQueryTool,
storageCoordinator,
auditManager
);
+ taskMaster.becomeHalfLeader(null, taskQueue);
taskMaster.becomeFullLeader();
storageCoordinator.commitSegments(Set.copyOf(WIKI_SEGMENTS_10X1D), null);
@@ -353,6 +368,50 @@ public class OverlordDataSourcesResourceTest
Assertions.assertEquals(expectedErrorMessage, response.getEntity());
}
+ @Test
+ public void testShutdownAllTasks_shutsDownEveryActiveTask()
+ {
+ final Task task1 = NoopTask.forDatasource(TestDataSource.WIKI);
+ final Task task2 = NoopTask.forDatasource(TestDataSource.WIKI);
+ EasyMock.expect(taskQueryTool.getActiveTaskInfo(TestDataSource.WIKI))
+ .andReturn(List.of(createActiveTaskInfo(task1),
createActiveTaskInfo(task2)));
+ taskQueue.shutdown(task1.getId(), "Shutdown request from user");
+ taskQueue.shutdown(task2.getId(), "Shutdown request from user");
+ EasyMock.replay(taskQueryTool, taskQueue);
+
+ final Response response =
dataSourcesResource.shutdownTasksForDataSource(TestDataSource.WIKI);
+ Assertions.assertEquals(200, response.getStatus());
+ Assertions.assertEquals(Map.of("dataSource", TestDataSource.WIKI),
response.getEntity());
+ EasyMock.verify(taskQueryTool, taskQueue);
+ }
+
+ @Test
+ public void testShutdownAllTasks_withNoActiveTasks_returnsNotFound()
+ {
+
EasyMock.expect(taskQueryTool.getActiveTaskInfo(TestDataSource.WIKI)).andReturn(List.of());
+ EasyMock.replay(taskQueryTool, taskQueue);
+
+ final Response response =
dataSourcesResource.shutdownTasksForDataSource(TestDataSource.WIKI);
+ Assertions.assertEquals(404, response.getStatus());
+ EasyMock.verify(taskQueryTool, taskQueue);
+ }
+
+ @Test
+ public void testShutdownAllTasks_whenNotLeader_returnsServiceUnavailable()
+ {
+ taskMaster.stopBeingLeader();
+ EasyMock.replay(taskQueryTool, taskQueue);
+
+ final Response response =
dataSourcesResource.shutdownTasksForDataSource(TestDataSource.WIKI);
+ Assertions.assertEquals(503, response.getStatus());
+ EasyMock.verify(taskQueryTool, taskQueue);
+ }
+
+ private static TaskInfo createActiveTaskInfo(Task task)
+ {
+ return new TaskInfo(DateTimes.nowUtc(), TaskStatus.running(task.getId()),
task);
+ }
+
private void verifyNumSegmentsUpdated(int expectedUpdatedCount, Response
response)
{
Assertions.assertEquals(200, response.getStatus());
diff --git
a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/http/OverlordResourceTest.java
b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/http/OverlordResourceTest.java
index ae1f50cc044..22d7cdf8acb 100644
---
a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/http/OverlordResourceTest.java
+++
b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/http/OverlordResourceTest.java
@@ -65,7 +65,6 @@ import org.apache.druid.java.util.common.Intervals;
import org.apache.druid.java.util.common.RE;
import org.apache.druid.java.util.common.UOE;
import org.apache.druid.metadata.LockFilterPolicy;
-import org.apache.druid.metadata.TaskLookup;
import org.apache.druid.metadata.TaskLookup.ActiveTaskLookup;
import org.apache.druid.metadata.TaskLookup.CompleteTaskLookup;
import org.apache.druid.metadata.TaskLookup.TaskLookupType;
@@ -1195,58 +1194,6 @@ public class OverlordResourceTest
Assertions.assertEquals("id_1", response.get("task"));
}
- @Test
- public void testShutdownAllTasks()
- {
- // This is disabled since OverlordResource.shutdownTasksForDataSource is
annotated with DatasourceResourceFilter
- // This should be fixed in https://github.com/apache/druid/issues/6685.
- // expectAuthorizationTokenCheck();
- TaskQueue mockQueue = EasyMock.createMock(TaskQueue.class);
- EasyMock.expect(taskMaster.getTaskQueue()).andReturn(
- Optional.of(mockQueue)
- ).anyTimes();
- EasyMock.expect(
- taskStorage.getTaskInfos(TaskLookup.activeTasksOnly(), "datasource")
- ).andStubReturn(ImmutableList.of(
- new TaskInfo(
- DateTime.now(ISOChronology.getInstanceUTC()),
- TaskStatus.success("id_1"),
- new NoopTask("id_1", null, "datasource", 1L, 0L, null)
- ),
- new TaskInfo(
- DateTime.now(ISOChronology.getInstanceUTC()),
- TaskStatus.success("id_2"),
- new NoopTask("id_2", null, "datasource", 1L, 0L, null)
- )
- ));
- mockQueue.shutdown("id_1", "Shutdown request from user");
- EasyMock.expectLastCall();
- mockQueue.shutdown("id_2", "Shutdown request from user");
- EasyMock.expectLastCall();
-
- replayAll();
- EasyMock.replay(mockQueue);
-
- final Map<String, String> response = (Map<String, String>) overlordResource
- .shutdownTasksForDataSource("datasource")
- .getEntity();
- Assertions.assertEquals("datasource", response.get("dataSource"));
- }
-
- @Test
- public void testShutdownAllTasksForNonExistingDataSource()
- {
- final TaskQueue taskQueue = EasyMock.createMock(TaskQueue.class);
- EasyMock.expect(overlord.isLeader()).andReturn(true).anyTimes();
-
EasyMock.expect(taskMaster.getTaskQueue()).andReturn(Optional.of(taskQueue)).anyTimes();
-
EasyMock.expect(taskStorage.getTaskInfos(EasyMock.anyObject(TaskLookup.class),
EasyMock.anyString()))
- .andReturn(Collections.emptyList());
- replayAll();
-
- final Response response =
overlordResource.shutdownTasksForDataSource("notExisting");
- Assertions.assertEquals(Status.NOT_FOUND.getStatusCode(),
response.getStatus());
- }
-
@Test
public void testEnableWorker()
{
diff --git a/services/pom.xml b/services/pom.xml
index 38eb1d03f17..4e38de2d764 100644
--- a/services/pom.xml
+++ b/services/pom.xml
@@ -170,6 +170,10 @@
<groupId>com.sun.jersey</groupId>
<artifactId>jersey-server</artifactId>
</dependency>
+ <dependency>
+ <groupId>com.sun.jersey</groupId>
+ <artifactId>jersey-core</artifactId>
+ </dependency>
<dependency>
<groupId>org.roaringbitmap</groupId>
diff --git
a/services/src/test/java/org/apache/druid/cli/JerseyShadowedEndpoints.java
b/services/src/test/java/org/apache/druid/cli/JerseyShadowedEndpoints.java
new file mode 100644
index 00000000000..e74f46312a3
--- /dev/null
+++ b/services/src/test/java/org/apache/druid/cli/JerseyShadowedEndpoints.java
@@ -0,0 +1,186 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.cli;
+
+import com.sun.jersey.api.model.AbstractMethod;
+import com.sun.jersey.api.model.AbstractResource;
+import com.sun.jersey.api.model.AbstractResourceMethod;
+import com.sun.jersey.api.model.AbstractSubResourceLocator;
+import com.sun.jersey.api.model.AbstractSubResourceMethod;
+import com.sun.jersey.server.impl.modelapi.annotation.IntrospectionModeller;
+import com.sun.jersey.server.impl.uri.PathPattern;
+import com.sun.jersey.server.impl.uri.PathTemplate;
+import org.apache.druid.java.util.common.ISE;
+import org.apache.druid.java.util.common.StringUtils;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+/**
+ * Finds Jersey endpoints that no request can reach, because the root resource
class of another endpoint shadows them.
+ * <p>
+ * Jersey 1.x routes a request in two steps. Out of the root resource classes
whose {@code @Path} matches the request
+ * path, it first picks the one that sorts first under {@link
PathPattern#COMPARATOR}, which prefers paths with more
+ * literal characters. It then looks for a matching method in that class only.
It never falls back to another class,
+ * so if the chosen class has no matching method, the request fails with a 404
even when another class declares that
+ * exact endpoint. For example, an endpoint at {@code
/druid/indexer/v1/datasources/{dataSource}/shutdownAllTasks} in a
+ * class with {@code @Path("/druid/indexer/v1")} stops working once another
class has
+ * {@code @Path("/druid/indexer/v1/datasources")}.
+ * <p>
+ * To apply exactly the same rules, this class uses Jersey's own resource
model and path matching.
+ */
+final class JerseyShadowedEndpoints
+{
+ /**
+ * Substituted for every path parameter to build a request path that reaches
an endpoint.
+ */
+ private static final String PATH_PARAMETER_VALUE = "x";
+
+ private JerseyShadowedEndpoints()
+ {
+ }
+
+ /**
+ * Returns a description of every shadowed endpoint of the given resource
classes, or an empty list if every endpoint
+ * is reachable. Classes without {@code @Path}, such as providers, are
ignored.
+ */
+ static List<String> find(final Collection<Class<?>> resourceClasses)
+ {
+ // In the order in which Jersey tries root resources.
+ final List<RootResource> rootResources =
+ resourceClasses.stream()
+ .map(IntrospectionModeller::createResource)
+ .filter(AbstractResource::isRootResource)
+ .map(RootResource::new)
+ .sorted((r1, r2) ->
PathPattern.COMPARATOR.compare(r1.pattern, r2.pattern))
+ .collect(Collectors.toList());
+
+ final List<String> shadowedEndpoints = new ArrayList<>();
+ for (final RootResource rootResource : rootResources) {
+ for (final Endpoint endpoint : rootResource.getEndpoints()) {
+ if (!rootResource.matches(endpoint.requestPath)) {
+ throw new ISE("Request path[%s] built for %s does not match its own
resource class", endpoint.requestPath, endpoint);
+ }
+ final RootResource routedTo = findRoutedResource(rootResources,
endpoint.requestPath);
+ if (routedTo != rootResource) {
+ shadowedEndpoints.add(StringUtils.format("%s is shadowed by %s",
endpoint, routedTo));
+ }
+ }
+ }
+ return shadowedEndpoints;
+ }
+
+ private static RootResource findRoutedResource(final List<RootResource>
sortedRootResources, final String requestPath)
+ {
+ for (final RootResource rootResource : sortedRootResources) {
+ if (rootResource.matches(requestPath)) {
+ return rootResource;
+ }
+ }
+ throw new ISE("No resource class matches request path[%s]", requestPath);
+ }
+
+ private static class RootResource
+ {
+ private final AbstractResource resource;
+ private final PathPattern pattern;
+
+ private RootResource(final AbstractResource resource)
+ {
+ this.resource = resource;
+ // Built the same way as in Jersey's RootResourceUriRules.
+ this.pattern = new PathPattern(new
PathTemplate(resource.getPath().getValue()));
+ }
+
+ private boolean matches(final String requestPath)
+ {
+ return pattern.match(requestPath) != null;
+ }
+
+ private List<Endpoint> getEndpoints()
+ {
+ final String path = resource.getPath().getValue();
+ final List<Endpoint> endpoints = new ArrayList<>();
+ for (final AbstractResourceMethod method :
resource.getResourceMethods()) {
+ endpoints.add(new Endpoint(method.getHttpMethod(), path, method));
+ }
+ for (final AbstractSubResourceMethod method :
resource.getSubResourceMethods()) {
+ endpoints.add(new Endpoint(method.getHttpMethod(), concatPaths(path,
method.getPath().getValue()), method));
+ }
+ for (final AbstractSubResourceLocator locator :
resource.getSubResourceLocators()) {
+ endpoints.add(new Endpoint("ANY", concatPaths(path,
locator.getPath().getValue()), locator));
+ }
+ return endpoints;
+ }
+
+ private static String concatPaths(final String path, final String subPath)
+ {
+ return StringUtils.maybeRemoveTrailingSlash(path) + "/" +
StringUtils.maybeRemoveLeadingSlash(subPath);
+ }
+
+ @Override
+ public String toString()
+ {
+ return StringUtils.format(
+ "[%s] with @Path[%s]",
+ resource.getResourceClass().getName(),
+ resource.getPath().getValue()
+ );
+ }
+ }
+
+ private static class Endpoint
+ {
+ private final String httpMethod;
+ private final String path;
+ private final AbstractMethod method;
+ private final String requestPath;
+
+ private Endpoint(final String httpMethod, final String path, final
AbstractMethod method)
+ {
+ this.httpMethod = httpMethod;
+ this.path = path;
+ this.method = method;
+
+ final PathTemplate template = new PathTemplate(path);
+ final Map<String, String> pathParameters = new HashMap<>();
+ for (final String name : template.getTemplateVariables()) {
+ pathParameters.put(name, PATH_PARAMETER_VALUE);
+ }
+ this.requestPath = template.createURI(pathParameters);
+ }
+
+ @Override
+ public String toString()
+ {
+ return StringUtils.format(
+ "[%s %s] of [%s.%s()]",
+ httpMethod,
+ path,
+ method.getMethod().getDeclaringClass().getName(),
+ method.getMethod().getName()
+ );
+ }
+ }
+}
diff --git
a/services/src/test/java/org/apache/druid/cli/JerseyShadowedEndpointsTest.java
b/services/src/test/java/org/apache/druid/cli/JerseyShadowedEndpointsTest.java
new file mode 100644
index 00000000000..80bbec0ac3c
--- /dev/null
+++
b/services/src/test/java/org/apache/druid/cli/JerseyShadowedEndpointsTest.java
@@ -0,0 +1,163 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.cli;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import javax.ws.rs.DELETE;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import java.util.List;
+
+public class JerseyShadowedEndpointsTest
+{
+ @Test
+ public void testEndpointUnderMoreSpecificPathOfAnotherResourceIsShadowed()
+ {
+ Assertions.assertEquals(
+ List.of(
+ "[POST
/druid/indexer/v1/datasources/{dataSource}/shutdownAllTasks]"
+ + " of [" + IndexerResource.class.getName() +
".shutdownAllTasks()]"
+ + " is shadowed by [" + DataSourcesResource.class.getName() + "]"
+ + " with @Path[/druid/indexer/v1/datasources]"
+ ),
+ JerseyShadowedEndpoints.find(List.of(IndexerResource.class,
DataSourcesResource.class))
+ );
+ }
+
+ @Test
+ public void testEndpointInMostSpecificResourceIsNotShadowed()
+ {
+ Assertions.assertEquals(
+ List.of(),
+ JerseyShadowedEndpoints.find(
+ List.of(IndexerResourceWithoutShutdown.class,
DataSourcesResourceWithShutdown.class, StatusResource.class)
+ )
+ );
+ }
+
+ @Test
+ public void
testSubResourceLocatorUnderMoreSpecificPathOfAnotherResourceIsShadowed()
+ {
+ Assertions.assertEquals(
+ List.of(
+ "[ANY /druid/indexer/v1/datasources/{dataSource}/handler]"
+ + " of [" + IndexerResourceWithLocator.class.getName() +
".getHandler()]"
+ + " is shadowed by [" + DataSourcesResource.class.getName() + "]"
+ + " with @Path[/druid/indexer/v1/datasources]"
+ ),
+ JerseyShadowedEndpoints.find(List.of(IndexerResourceWithLocator.class,
DataSourcesResource.class))
+ );
+ }
+
+ @Test
+ public void testClassWithoutPathIsIgnored()
+ {
+ Assertions.assertEquals(
+ List.of(),
+ JerseyShadowedEndpoints.find(List.of(NotAResource.class,
StatusResource.class))
+ );
+ }
+
+ /**
+ * Same layout as {@code OverlordResource} and {@code
OverlordDataSourcesResource} had before the
+ * {@code shutdownAllTasks} endpoint moved.
+ */
+ @Path("/druid/indexer/v1")
+ public static class IndexerResource
+ {
+ @GET
+ @Path("/task/{taskId}")
+ public void getTask()
+ {
+ }
+
+ @POST
+ @Path("/datasources/{dataSource}/shutdownAllTasks")
+ public void shutdownAllTasks()
+ {
+ }
+ }
+
+ @Path("/druid/indexer/v1/datasources")
+ public static class DataSourcesResource
+ {
+ @DELETE
+ @Path("/{dataSource}")
+ public void markAllSegmentsAsUnused()
+ {
+ }
+ }
+
+ @Path("/druid/indexer/v1")
+ public static class IndexerResourceWithoutShutdown
+ {
+ @GET
+ @Path("/task/{taskId}")
+ public void getTask()
+ {
+ }
+ }
+
+ @Path("/druid/indexer/v1/datasources")
+ public static class DataSourcesResourceWithShutdown
+ {
+ @DELETE
+ @Path("/{dataSource}")
+ public void markAllSegmentsAsUnused()
+ {
+ }
+
+ @POST
+ @Path("/{dataSource}/shutdownAllTasks")
+ public void shutdownAllTasks()
+ {
+ }
+ }
+
+ @Path("/druid/indexer/v1")
+ public static class IndexerResourceWithLocator
+ {
+ @Path("/datasources/{dataSource}/handler")
+ public Object getHandler()
+ {
+ return new Object();
+ }
+ }
+
+ @Path("/status")
+ public static class StatusResource
+ {
+ @GET
+ public void getStatus()
+ {
+ }
+ }
+
+ public static class NotAResource
+ {
+ @GET
+ public void get()
+ {
+ }
+ }
+}
diff --git a/services/src/test/java/org/apache/druid/cli/MainTest.java
b/services/src/test/java/org/apache/druid/cli/MainTest.java
index 33fb605d1f6..229855dcc67 100644
--- a/services/src/test/java/org/apache/druid/cli/MainTest.java
+++ b/services/src/test/java/org/apache/druid/cli/MainTest.java
@@ -20,12 +20,17 @@
package org.apache.druid.cli;
import com.google.inject.Injector;
+import com.google.inject.Key;
+import com.google.inject.TypeLiteral;
import org.apache.druid.guice.GuiceInjectors;
+import org.apache.druid.guice.annotations.JSR311Resource;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
+import java.util.List;
import java.util.Properties;
+import java.util.Set;
import java.util.stream.Stream;
public class MainTest
@@ -47,8 +52,33 @@ public class MainTest
@MethodSource("runnables")
public void testSimpleInjection(ServerRunnable runnable)
{
- final Injector injector = GuiceInjectors.makeStartupInjector();
- injector.injectMembers(runnable);
- Assertions.assertNotNull(runnable.makeInjector(runnable.getNodeRoles(new
Properties())));
+ Assertions.assertNotNull(makeInjector(runnable));
+ }
+
+ /**
+ * Jersey routes each request to a single root resource class, so an
endpoint is unreachable if its path falls under
+ * a more specific {@code @Path} of another resource class served by the
same process.
+ */
+ @ParameterizedTest(name = "{0}")
+ @MethodSource("runnables")
+ public void testNoShadowedEndpoints(final ServerRunnable runnable)
+ {
+ final Set<Class<?>> resources = makeInjector(runnable).getInstance(
+ Key.get(new TypeLiteral<Set<Class<?>>>() {}, JSR311Resource.class)
+ );
+ Assertions.assertFalse(resources.isEmpty());
+
+ final List<String> shadowedEndpoints =
JerseyShadowedEndpoints.find(resources);
+ Assertions.assertTrue(
+ shadowedEndpoints.isEmpty(),
+ () -> "Unreachable endpoints:\n" + String.join("\n", shadowedEndpoints)
+ );
+ }
+
+ private static Injector makeInjector(final ServerRunnable runnable)
+ {
+ final Injector startupInjector = GuiceInjectors.makeStartupInjector();
+ startupInjector.injectMembers(runnable);
+ return runnable.makeInjector(runnable.getNodeRoles(new Properties()));
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
