imbajin commented on code in PR #2898:
URL:
https://github.com/apache/incubator-hugegraph/pull/2898#discussion_r2489990233
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/PathFilter.java:
##########
@@ -18,41 +18,101 @@
package org.apache.hugegraph.api.filter;
import java.io.IOException;
+import java.net.URI;
+import java.util.List;
+import java.util.Set;
+
+import org.apache.hugegraph.config.HugeConfig;
+import org.apache.hugegraph.config.ServerOptions;
+import org.apache.hugegraph.util.E;
+import org.apache.hugegraph.util.Log;
+import org.slf4j.Logger;
+
+import com.google.common.collect.ImmutableSet;
import jakarta.inject.Singleton;
import jakarta.ws.rs.container.ContainerRequestContext;
import jakarta.ws.rs.container.ContainerRequestFilter;
import jakarta.ws.rs.container.PreMatching;
+import jakarta.ws.rs.core.Context;
+import jakarta.ws.rs.core.PathSegment;
+import jakarta.ws.rs.core.UriInfo;
import jakarta.ws.rs.ext.Provider;
@Provider
@Singleton
@PreMatching
public class PathFilter implements ContainerRequestFilter {
+ private static final Logger LOG = Log.logger(PathFilter.class);
+
+ private static final String GRAPH_SPACE = "graphspaces";
+ private static final String ARTHAS_START = "arthas";
+
public static final String REQUEST_TIME = "request_time";
public static final String REQUEST_PARAMS_JSON = "request_params_json";
+ private static final String DELIMITER = "/";
+ private static final Set<String> WHITE_API_LIST = ImmutableSet.of(
+ "",
+ "apis",
+ "metrics",
+ "versions",
+ "health",
+ "gremlin",
+ "graphs/auth",
+ "graphs/auth/users",
+ "auth/users",
+ "auth/managers",
+ "auth",
+ "hstore",
+ "pd",
+ "kafka",
+ "whiteiplist",
+ "vermeer",
+ "store",
+ "expiredclear",
+ "department",
+ "saas",
+ "trade",
+ "kvstore",
+ "openapi.json"
+ );
+
+ @Context
+ private jakarta.inject.Provider<HugeConfig> configProvider;
+
+ public static boolean isWhiteAPI(String rootPath) {
+
+ return WHITE_API_LIST.contains(rootPath);
+ }
+
@Override
- public void filter(ContainerRequestContext context) throws IOException {
+ public void filter(ContainerRequestContext context)
+ throws IOException {
context.setProperty(REQUEST_TIME, System.currentTimeMillis());
- // TODO: temporarily comment it to fix loader bug, handle it later
- /*// record the request json
- String method = context.getMethod();
- String requestParamsJson = "";
- if (method.equals(HttpMethod.POST)) {
- requestParamsJson = IOUtils.toString(context.getEntityStream(),
- Charsets.toCharset(CHARSET));
- // replace input stream because we have already read it
- InputStream in = IOUtils.toInputStream(requestParamsJson,
Charsets.toCharset(CHARSET));
- context.setEntityStream(in);
- } else if (method.equals(HttpMethod.GET)) {
- MultivaluedMap<String, String> pathParameters =
context.getUriInfo()
-
.getPathParameters();
- requestParamsJson = pathParameters.toString();
+ List<PathSegment> segments = context.getUriInfo().getPathSegments();
+ E.checkArgument(segments.size() > 0, "Invalid request uri '%s'",
+ context.getUriInfo().getPath());
+ String rootPath = segments.get(0).getPath();
+
+ if (isWhiteAPI(rootPath) || GRAPH_SPACE.equals(rootPath) ||
+ ARTHAS_START.equals(rootPath)) {
+ return;
}
- context.setProperty(REQUEST_PARAMS_JSON, requestParamsJson);*/
+ UriInfo uriInfo = context.getUriInfo();
+ String defaultPathSpace =
+ this.configProvider.get().get(ServerOptions.PATH_GRAPH_SPACE);
+ String path = uriInfo.getBaseUri().getPath() +
+ String.join(DELIMITER, GRAPH_SPACE, defaultPathSpace);
+ for (PathSegment segment : segments) {
Review Comment:
‼️ **路径重定向逻辑存在严重问题**
当前实现中,对于非白名单路径,会将原始路径重定向到 `/graphspaces/{DEFAULT}/` 前缀下。但这个逻辑存在问题:
1. **路径拼接逻辑错误**: 在第107-110行,代码将 `baseUri.getPath()` 和 `GRAPH_SPACE` 以及所有
segments 拼接在一起,这会导致路径重复或错误。`baseUri.getPath()` 通常已经包含了基础路径,再拼接会导致路径不正确。
2. **建议修改**:
```java
String path = String.join(DELIMITER, GRAPH_SPACE, defaultPathSpace);
for (PathSegment segment : segments) {
path = String.join(DELIMITER, path, segment.getPath());
}
```
这样可以避免 baseUri 路径的重复拼接问题。
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/PathFilter.java:
##########
@@ -18,41 +18,101 @@
package org.apache.hugegraph.api.filter;
import java.io.IOException;
+import java.net.URI;
+import java.util.List;
+import java.util.Set;
+
+import org.apache.hugegraph.config.HugeConfig;
+import org.apache.hugegraph.config.ServerOptions;
+import org.apache.hugegraph.util.E;
+import org.apache.hugegraph.util.Log;
+import org.slf4j.Logger;
+
+import com.google.common.collect.ImmutableSet;
import jakarta.inject.Singleton;
import jakarta.ws.rs.container.ContainerRequestContext;
import jakarta.ws.rs.container.ContainerRequestFilter;
import jakarta.ws.rs.container.PreMatching;
+import jakarta.ws.rs.core.Context;
+import jakarta.ws.rs.core.PathSegment;
+import jakarta.ws.rs.core.UriInfo;
import jakarta.ws.rs.ext.Provider;
@Provider
@Singleton
@PreMatching
public class PathFilter implements ContainerRequestFilter {
+ private static final Logger LOG = Log.logger(PathFilter.class);
+
+ private static final String GRAPH_SPACE = "graphspaces";
+ private static final String ARTHAS_START = "arthas";
+
public static final String REQUEST_TIME = "request_time";
public static final String REQUEST_PARAMS_JSON = "request_params_json";
+ private static final String DELIMITER = "/";
+ private static final Set<String> WHITE_API_LIST = ImmutableSet.of(
+ "",
+ "apis",
+ "metrics",
+ "versions",
+ "health",
+ "gremlin",
Review Comment:
‼️ **白名单设计存在歧义**
当前白名单中同时包含单段路径(如 `auth`)和多段路径字符串(如 `auth/users`, `graphs/auth/users`)。但是在
`filter` 方法中,只检查第一个 segment:
```java
String rootPath = segments.get(0).getPath();
if (isWhiteAPI(rootPath) || ...)
```
这意味着:
- `/auth/xxx` 会被跳过(因为第一段是 `auth`)
- 白名单中的 `auth/users` 和 `graphs/auth/users` 这些多段条目永远不会被匹配到,因为 `isWhiteAPI`
接收的只是第一个 segment
**建议**:
1. 如果只需要匹配第一段,则移除白名单中的多段路径条目(`graphs/auth`, `graphs/auth/users`,
`auth/users`)
2. 如果需要精确匹配完整路径,则需要修改匹配逻辑,传入完整路径而非仅第一段
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/PathFilter.java:
##########
@@ -18,41 +18,101 @@
package org.apache.hugegraph.api.filter;
import java.io.IOException;
+import java.net.URI;
+import java.util.List;
+import java.util.Set;
+
+import org.apache.hugegraph.config.HugeConfig;
+import org.apache.hugegraph.config.ServerOptions;
+import org.apache.hugegraph.util.E;
+import org.apache.hugegraph.util.Log;
+import org.slf4j.Logger;
+
+import com.google.common.collect.ImmutableSet;
import jakarta.inject.Singleton;
import jakarta.ws.rs.container.ContainerRequestContext;
import jakarta.ws.rs.container.ContainerRequestFilter;
import jakarta.ws.rs.container.PreMatching;
+import jakarta.ws.rs.core.Context;
+import jakarta.ws.rs.core.PathSegment;
+import jakarta.ws.rs.core.UriInfo;
import jakarta.ws.rs.ext.Provider;
@Provider
@Singleton
@PreMatching
public class PathFilter implements ContainerRequestFilter {
+ private static final Logger LOG = Log.logger(PathFilter.class);
+
+ private static final String GRAPH_SPACE = "graphspaces";
+ private static final String ARTHAS_START = "arthas";
+
public static final String REQUEST_TIME = "request_time";
public static final String REQUEST_PARAMS_JSON = "request_params_json";
+ private static final String DELIMITER = "/";
+ private static final Set<String> WHITE_API_LIST = ImmutableSet.of(
+ "",
+ "apis",
+ "metrics",
+ "versions",
+ "health",
+ "gremlin",
+ "graphs/auth",
+ "graphs/auth/users",
+ "auth/users",
+ "auth/managers",
+ "auth",
+ "hstore",
+ "pd",
+ "kafka",
+ "whiteiplist",
+ "vermeer",
+ "store",
+ "expiredclear",
+ "department",
+ "saas",
+ "trade",
+ "kvstore",
+ "openapi.json"
+ );
+
+ @Context
+ private jakarta.inject.Provider<HugeConfig> configProvider;
+
+ public static boolean isWhiteAPI(String rootPath) {
+
+ return WHITE_API_LIST.contains(rootPath);
+ }
+
@Override
- public void filter(ContainerRequestContext context) throws IOException {
+ public void filter(ContainerRequestContext context)
+ throws IOException {
context.setProperty(REQUEST_TIME, System.currentTimeMillis());
- // TODO: temporarily comment it to fix loader bug, handle it later
- /*// record the request json
- String method = context.getMethod();
- String requestParamsJson = "";
- if (method.equals(HttpMethod.POST)) {
- requestParamsJson = IOUtils.toString(context.getEntityStream(),
- Charsets.toCharset(CHARSET));
- // replace input stream because we have already read it
- InputStream in = IOUtils.toInputStream(requestParamsJson,
Charsets.toCharset(CHARSET));
- context.setEntityStream(in);
- } else if (method.equals(HttpMethod.GET)) {
- MultivaluedMap<String, String> pathParameters =
context.getUriInfo()
-
.getPathParameters();
- requestParamsJson = pathParameters.toString();
+ List<PathSegment> segments = context.getUriInfo().getPathSegments();
+ E.checkArgument(segments.size() > 0, "Invalid request uri '%s'",
Review Comment:
⚠️ **异常处理不完善**
第95-96行对路径段数量进行检查:
```java
E.checkArgument(segments.size() > 0, "Invalid request uri '%s'",
context.getUriInfo().getPath());
```
但这个检查可能不够:
1. JAX-RS 规范中,根路径 `/` 可能返回一个包含空字符串的 segment list
2. 应该同时检查第一个 segment 是否为 null 或空字符串
**建议**:
```java
E.checkArgument(segments != null && !segments.isEmpty(),
"Invalid request uri '%s'", context.getUriInfo().getPath());
String rootPath = segments.get(0).getPath();
E.checkArgument(rootPath != null, "Invalid root path in uri '%s'",
context.getUriInfo().getPath());
```
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/PathFilter.java:
##########
@@ -18,41 +18,101 @@
package org.apache.hugegraph.api.filter;
import java.io.IOException;
+import java.net.URI;
+import java.util.List;
+import java.util.Set;
+
+import org.apache.hugegraph.config.HugeConfig;
+import org.apache.hugegraph.config.ServerOptions;
+import org.apache.hugegraph.util.E;
+import org.apache.hugegraph.util.Log;
+import org.slf4j.Logger;
+
+import com.google.common.collect.ImmutableSet;
import jakarta.inject.Singleton;
import jakarta.ws.rs.container.ContainerRequestContext;
import jakarta.ws.rs.container.ContainerRequestFilter;
import jakarta.ws.rs.container.PreMatching;
+import jakarta.ws.rs.core.Context;
+import jakarta.ws.rs.core.PathSegment;
+import jakarta.ws.rs.core.UriInfo;
import jakarta.ws.rs.ext.Provider;
@Provider
@Singleton
@PreMatching
public class PathFilter implements ContainerRequestFilter {
+ private static final Logger LOG = Log.logger(PathFilter.class);
+
+ private static final String GRAPH_SPACE = "graphspaces";
+ private static final String ARTHAS_START = "arthas";
+
public static final String REQUEST_TIME = "request_time";
public static final String REQUEST_PARAMS_JSON = "request_params_json";
+ private static final String DELIMITER = "/";
+ private static final Set<String> WHITE_API_LIST = ImmutableSet.of(
+ "",
+ "apis",
+ "metrics",
+ "versions",
+ "health",
+ "gremlin",
+ "graphs/auth",
+ "graphs/auth/users",
+ "auth/users",
+ "auth/managers",
+ "auth",
+ "hstore",
+ "pd",
+ "kafka",
+ "whiteiplist",
+ "vermeer",
+ "store",
+ "expiredclear",
+ "department",
+ "saas",
+ "trade",
+ "kvstore",
+ "openapi.json"
+ );
+
+ @Context
+ private jakarta.inject.Provider<HugeConfig> configProvider;
+
+ public static boolean isWhiteAPI(String rootPath) {
+
+ return WHITE_API_LIST.contains(rootPath);
+ }
+
@Override
- public void filter(ContainerRequestContext context) throws IOException {
+ public void filter(ContainerRequestContext context)
+ throws IOException {
context.setProperty(REQUEST_TIME, System.currentTimeMillis());
- // TODO: temporarily comment it to fix loader bug, handle it later
- /*// record the request json
- String method = context.getMethod();
- String requestParamsJson = "";
- if (method.equals(HttpMethod.POST)) {
- requestParamsJson = IOUtils.toString(context.getEntityStream(),
- Charsets.toCharset(CHARSET));
- // replace input stream because we have already read it
- InputStream in = IOUtils.toInputStream(requestParamsJson,
Charsets.toCharset(CHARSET));
- context.setEntityStream(in);
- } else if (method.equals(HttpMethod.GET)) {
- MultivaluedMap<String, String> pathParameters =
context.getUriInfo()
-
.getPathParameters();
- requestParamsJson = pathParameters.toString();
+ List<PathSegment> segments = context.getUriInfo().getPathSegments();
+ E.checkArgument(segments.size() > 0, "Invalid request uri '%s'",
+ context.getUriInfo().getPath());
+ String rootPath = segments.get(0).getPath();
+
+ if (isWhiteAPI(rootPath) || GRAPH_SPACE.equals(rootPath) ||
+ ARTHAS_START.equals(rootPath)) {
+ return;
}
- context.setProperty(REQUEST_PARAMS_JSON, requestParamsJson);*/
+ UriInfo uriInfo = context.getUriInfo();
+ String defaultPathSpace =
+ this.configProvider.get().get(ServerOptions.PATH_GRAPH_SPACE);
+ String path = uriInfo.getBaseUri().getPath() +
+ String.join(DELIMITER, GRAPH_SPACE, defaultPathSpace);
+ for (PathSegment segment : segments) {
+ path = String.join(DELIMITER, path, segment.getPath());
+ }
+ LOG.debug("Redirect request uri from {} to {}",
+ uriInfo.getRequestUri().getPath(), path);
Review Comment:
⚠️ **URI 重建逻辑可能丢失查询参数**
在第114行:
```java
URI requestUri = uriInfo.getRequestUriBuilder().uri(path).build();
```
这里使用 `.uri(path)` 可能会覆盖原有的查询参数。应该使用 `replacePath()` 方法来保留查询参数:
```suggestion
URI requestUri = uriInfo.getRequestUriBuilder().replacePath(path).build();
```
或者确认测试用例中查询参数确实被保留(虽然测试中有验证,但实现方式需要确保正确性)。
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/PathFilter.java:
##########
@@ -18,41 +18,101 @@
package org.apache.hugegraph.api.filter;
import java.io.IOException;
+import java.net.URI;
+import java.util.List;
+import java.util.Set;
+
+import org.apache.hugegraph.config.HugeConfig;
+import org.apache.hugegraph.config.ServerOptions;
+import org.apache.hugegraph.util.E;
+import org.apache.hugegraph.util.Log;
+import org.slf4j.Logger;
+
+import com.google.common.collect.ImmutableSet;
import jakarta.inject.Singleton;
import jakarta.ws.rs.container.ContainerRequestContext;
import jakarta.ws.rs.container.ContainerRequestFilter;
import jakarta.ws.rs.container.PreMatching;
+import jakarta.ws.rs.core.Context;
+import jakarta.ws.rs.core.PathSegment;
+import jakarta.ws.rs.core.UriInfo;
import jakarta.ws.rs.ext.Provider;
@Provider
@Singleton
@PreMatching
public class PathFilter implements ContainerRequestFilter {
+ private static final Logger LOG = Log.logger(PathFilter.class);
+
+ private static final String GRAPH_SPACE = "graphspaces";
+ private static final String ARTHAS_START = "arthas";
+
public static final String REQUEST_TIME = "request_time";
public static final String REQUEST_PARAMS_JSON = "request_params_json";
+ private static final String DELIMITER = "/";
+ private static final Set<String> WHITE_API_LIST = ImmutableSet.of(
+ "",
+ "apis",
+ "metrics",
+ "versions",
+ "health",
+ "gremlin",
+ "graphs/auth",
+ "graphs/auth/users",
+ "auth/users",
+ "auth/managers",
+ "auth",
+ "hstore",
+ "pd",
+ "kafka",
+ "whiteiplist",
+ "vermeer",
+ "store",
+ "expiredclear",
+ "department",
+ "saas",
+ "trade",
+ "kvstore",
+ "openapi.json"
+ );
+
+ @Context
+ private jakarta.inject.Provider<HugeConfig> configProvider;
+
+ public static boolean isWhiteAPI(String rootPath) {
+
+ return WHITE_API_LIST.contains(rootPath);
+ }
+
@Override
- public void filter(ContainerRequestContext context) throws IOException {
+ public void filter(ContainerRequestContext context)
+ throws IOException {
context.setProperty(REQUEST_TIME, System.currentTimeMillis());
- // TODO: temporarily comment it to fix loader bug, handle it later
- /*// record the request json
- String method = context.getMethod();
- String requestParamsJson = "";
- if (method.equals(HttpMethod.POST)) {
- requestParamsJson = IOUtils.toString(context.getEntityStream(),
- Charsets.toCharset(CHARSET));
- // replace input stream because we have already read it
- InputStream in = IOUtils.toInputStream(requestParamsJson,
Charsets.toCharset(CHARSET));
- context.setEntityStream(in);
- } else if (method.equals(HttpMethod.GET)) {
- MultivaluedMap<String, String> pathParameters =
context.getUriInfo()
-
.getPathParameters();
- requestParamsJson = pathParameters.toString();
+ List<PathSegment> segments = context.getUriInfo().getPathSegments();
+ E.checkArgument(segments.size() > 0, "Invalid request uri '%s'",
+ context.getUriInfo().getPath());
+ String rootPath = segments.get(0).getPath();
+
+ if (isWhiteAPI(rootPath) || GRAPH_SPACE.equals(rootPath) ||
+ ARTHAS_START.equals(rootPath)) {
+ return;
}
- context.setProperty(REQUEST_PARAMS_JSON, requestParamsJson);*/
+ UriInfo uriInfo = context.getUriInfo();
+ String defaultPathSpace =
+ this.configProvider.get().get(ServerOptions.PATH_GRAPH_SPACE);
+ String path = uriInfo.getBaseUri().getPath() +
+ String.join(DELIMITER, GRAPH_SPACE, defaultPathSpace);
+ for (PathSegment segment : segments) {
+ path = String.join(DELIMITER, path, segment.getPath());
+ }
Review Comment:
⚠️ **日志级别可能不合适**
第112行使用 DEBUG 级别记录路径重定向:
```java
LOG.debug("Redirect request uri from {} to {}",
uriInfo.getRequestUri().getPath(), path);
```
考虑到这是一个核心的路径重定向功能,建议:
1. 使用 INFO 级别,以便在生产环境中追踪路径重写行为
2. 或者提供配置选项来控制是否记录重定向日志
路径重定向是重要的路由决策,应该有适当的可观测性。
##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/filter/PathFilterTest.java:
##########
@@ -0,0 +1,414 @@
+/*
+ * 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.hugegraph.unit.api.filter;
+
+import java.io.IOException;
+import java.net.URI;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+import org.apache.commons.configuration2.Configuration;
+import org.apache.commons.configuration2.PropertiesConfiguration;
+import org.apache.hugegraph.api.filter.PathFilter;
+import org.apache.hugegraph.config.HugeConfig;
+import org.apache.hugegraph.config.ServerOptions;
+import org.apache.hugegraph.testutil.Assert;
+import org.apache.hugegraph.unit.BaseUnitTest;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mockito;
+
+import jakarta.inject.Provider;
+import jakarta.ws.rs.container.ContainerRequestContext;
+import jakarta.ws.rs.core.PathSegment;
+import jakarta.ws.rs.core.UriBuilder;
+import jakarta.ws.rs.core.UriInfo;
+
+/**
+ * Unit tests for PathFilter
+ * Test scenarios:
+ * 1. Whitelist paths are not redirected
+ * 2. Normal paths are correctly prefixed with graphspace
+ * 3. Query parameters are preserved
+ * 4. Special characters and encoding handling
+ * 5. Edge cases (empty path, root path, etc.)
+ */
+public class PathFilterTest extends BaseUnitTest {
+
+ private PathFilter pathFilter;
+ private Provider<HugeConfig> configProvider;
+ private HugeConfig config;
+ private ContainerRequestContext requestContext;
+ private UriInfo uriInfo;
+
+ @Before
+ public void setup() {
+ // Create configuration
+ Configuration conf = new PropertiesConfiguration();
+ conf.setProperty(ServerOptions.PATH_GRAPH_SPACE.name(), "DEFAULT");
+ this.config = new HugeConfig(conf);
+
+ // Create Provider
+ this.configProvider = () -> config;
+
+ // Create PathFilter and inject Provider
+ this.pathFilter = new PathFilter();
+ injectProvider(this.pathFilter, this.configProvider);
+
+ // Mock request context and uriInfo
+ this.requestContext = Mockito.mock(ContainerRequestContext.class);
+ this.uriInfo = Mockito.mock(UriInfo.class);
+
Mockito.when(this.requestContext.getUriInfo()).thenReturn(this.uriInfo);
+ }
+
+ /**
+ * Inject configProvider using reflection
+ */
+ private void injectProvider(PathFilter filter, Provider<HugeConfig>
provider) {
+ try {
+ java.lang.reflect.Field field =
PathFilter.class.getDeclaredField("configProvider");
+ field.setAccessible(true);
+ field.set(filter, provider);
+ } catch (Exception e) {
+ throw new RuntimeException("Failed to inject configProvider", e);
+ }
+ }
+
+ /**
+ * Create PathSegment mock
+ */
+ private PathSegment createPathSegment(String path) {
+ PathSegment segment = Mockito.mock(PathSegment.class);
+ Mockito.when(segment.getPath()).thenReturn(path);
+ return segment;
+ }
+
+ /**
+ * Setup URI information
+ */
+ private void setupUriInfo(String basePath, String requestPath,
List<String> segments,
+ String query) {
+ URI baseUri = URI.create("http://localhost:8080"; + basePath);
+ URI requestUri = query != null ? URI.create("http://localhost:8080"; +
requestPath + "?" + query) : URI.create("http://localhost:8080"; + requestPath);
+
+ Mockito.when(uriInfo.getBaseUri()).thenReturn(baseUri);
+ Mockito.when(uriInfo.getRequestUri()).thenReturn(requestUri);
+
+ List<PathSegment> pathSegments = new ArrayList<>();
+ for (String segment : segments) {
+ pathSegments.add(createPathSegment(segment));
+ }
+ Mockito.when(uriInfo.getPathSegments()).thenReturn(pathSegments);
+ Mockito.when(uriInfo.getPath()).thenReturn(String.join("/", segments));
+
+ // Mock UriBuilder - capture the path passed to uri() method
+ final String[] capturedPath = new String[1];
+ UriBuilder uriBuilder = Mockito.mock(UriBuilder.class);
+ Mockito.when(uriInfo.getRequestUriBuilder()).thenReturn(uriBuilder);
+
Mockito.when(uriBuilder.uri(Mockito.anyString())).thenAnswer(invocation -> {
+ capturedPath[0] = invocation.getArgument(0);
+ return uriBuilder;
+ });
+ Mockito.when(uriBuilder.build()).thenAnswer(invocation -> {
+ // Build URI based on captured path and preserve query parameters
+ String path = capturedPath[0] != null ? capturedPath[0] :
requestPath;
+ return URI.create("http://localhost:8080"; + path + (query != null
? "?" + query : ""));
+ });
+ }
+
+ /**
+ * Test whitelist API - empty path
+ */
+ @Test
+ public void testWhiteListApi_EmptyPath() throws IOException {
+ setupUriInfo("/", "/", List.of(""), null);
+
+ pathFilter.filter(requestContext);
+
+ // Verify whitelist API does not trigger setRequestUri
+ Mockito.verify(requestContext, Mockito.never()).setRequestUri(
+ Mockito.any(URI.class), Mockito.any(URI.class));
+ // Verify request timestamp is set
+ Mockito.verify(requestContext).setProperty(
+ Mockito.eq(PathFilter.REQUEST_TIME), Mockito.anyLong());
+ }
+
+ /**
+ * Test whitelist API - /apis
+ */
+ @Test
+ public void testWhiteListApi_Apis() throws IOException {
+ setupUriInfo("/", "/apis", List.of("apis"), null);
+
+ pathFilter.filter(requestContext);
+
+ Mockito.verify(requestContext, Mockito.never()).setRequestUri(
+ Mockito.any(URI.class), Mockito.any(URI.class));
+ }
+
+
+ /**
+ * Test whitelist API - /gremlin
+ */
+ @Test
+ public void testWhiteListApi_Gremlin() throws IOException {
+ setupUriInfo("/", "/gremlin", List.of("gremlin"), null);
+
+ pathFilter.filter(requestContext);
+
+ Mockito.verify(requestContext, Mockito.never()).setRequestUri(
+ Mockito.any(URI.class), Mockito.any(URI.class));
+ }
+
+ /**
+ * Test whitelist API - /auth (single segment)
+ */
+ @Test
+ public void testWhiteListApi_Auth() throws IOException {
+ setupUriInfo("/", "/auth", List.of("auth"), null);
+
+ pathFilter.filter(requestContext);
+
+ Mockito.verify(requestContext,
Mockito.never()).setRequestUri(Mockito.any(URI.class), Mockito.any(URI.class));
+ }
+
+ /**
+ * Test whitelist API - /auth/users (multi-segment path)
+ */
+ @Test
+ public void testWhiteListApi_AuthUsers_MultiSegment() throws IOException {
+ // Test complete /auth/users path with all segments
+ setupUriInfo("/", "/auth/users", Arrays.asList("auth", "users"), null);
+
+ pathFilter.filter(requestContext);
+
+ // Should not be redirected (first segment "auth" matches whitelist)
+ Mockito.verify(requestContext, Mockito.never()).setRequestUri(
+ Mockito.any(URI.class), Mockito.any(URI.class));
+ }
+
+ /**
+ * Test graphspaces path is not redirected
+ */
+ @Test
+ public void testGraphSpacePath_NotRedirected() throws IOException {
+ setupUriInfo("/", "/graphspaces/space1/graphs",
Arrays.asList("graphspaces", "space1", "graphs"), null);
+
+ pathFilter.filter(requestContext);
+
+ Mockito.verify(requestContext, Mockito.never()).setRequestUri(
+ Mockito.any(URI.class), Mockito.any(URI.class));
+ }
+
+ /**
+ * Test arthas path is not redirected
+ */
+ @Test
+ public void testArthasPath_NotRedirected() throws IOException {
+ setupUriInfo("/", "/arthas/api", Arrays.asList("arthas", "api"), null);
+
+ pathFilter.filter(requestContext);
+
+ Mockito.verify(requestContext, Mockito.never()).setRequestUri(
+ Mockito.any(URI.class), Mockito.any(URI.class));
+ }
+
+ /**
+ * Test normal path is correctly redirected - single segment
+ */
+ @Test
+ public void testNormalPath_SingleSegment() throws IOException {
+ setupUriInfo("/", "/graphs", List.of("graphs"), null);
+
+ pathFilter.filter(requestContext);
+
+ // Verify redirect is called with correct path
+ ArgumentCaptor<URI> uriCaptor = ArgumentCaptor.forClass(URI.class);
+ Mockito.verify(requestContext).setRequestUri(Mockito.any(URI.class),
uriCaptor.capture());
+
+ URI capturedUri = uriCaptor.getValue();
+ Assert.assertTrue("Redirect URI should contain graphspaces/DEFAULT
prefix", capturedUri.getPath().startsWith("/graphspaces/DEFAULT/graphs"));
+ Assert.assertEquals("/graphspaces/DEFAULT/graphs",
capturedUri.getPath());
+ }
+
+ /**
+ * Test normal path is correctly redirected - multiple segments
+ */
+ @Test
+ public void testNormalPath_MultipleSegments() throws IOException {
+ setupUriInfo("/", "/graphs/hugegraph/vertices",
Arrays.asList("graphs", "hugegraph", "vertices"), null);
+
+ pathFilter.filter(requestContext);
+
+ // Verify redirect is called with correct path
+ ArgumentCaptor<URI> uriCaptor = ArgumentCaptor.forClass(URI.class);
+ Mockito.verify(requestContext).setRequestUri(Mockito.any(URI.class),
uriCaptor.capture());
+
+ URI capturedUri = uriCaptor.getValue();
+ Assert.assertEquals("/graphspaces/DEFAULT/graphs/hugegraph/vertices",
capturedUri.getPath());
+ }
+
+ /**
+ * Test query parameters are preserved
+ */
+ @Test
+ public void testQueryParameters_Preserved() throws IOException {
+ String queryString = "limit=10&offset=20&label=person";
+ setupUriInfo("/", "/graphs/hugegraph/vertices",
Arrays.asList("graphs", "hugegraph", "vertices"), queryString);
+
+ URI originalRequestUri = uriInfo.getRequestUri();
+ Assert.assertTrue("Original URI should contain query string",
originalRequestUri.toString().contains(queryString));
+
+ pathFilter.filter(requestContext);
+
+ // Use ArgumentCaptor to capture the actual URI passed to setRequestUri
+ ArgumentCaptor<URI> uriCaptor = ArgumentCaptor.forClass(URI.class);
+ Mockito.verify(requestContext).setRequestUri(Mockito.any(URI.class),
uriCaptor.capture());
+
+ URI capturedUri = uriCaptor.getValue();
+ // Verify query parameters are indeed preserved
+ Assert.assertNotNull("Query parameters should be preserved",
capturedUri.getQuery());
+ Assert.assertTrue("Query should contain limit parameter",
capturedUri.getQuery().contains("limit=10"));
+ Assert.assertTrue("Query should contain offset parameter",
capturedUri.getQuery().contains("offset=20"));
+ Assert.assertTrue("Query should contain label parameter",
capturedUri.getQuery().contains("label=person"));
+ }
+
+ /**
+ * Test special characters in path handling
+ */
+ @Test
+ public void testSpecialCharacters_InPath() throws IOException {
+ setupUriInfo("/", "/schema/vertexlabels/person-label",
Arrays.asList("schema", "vertexlabels", "person-label"), null);
+
+ pathFilter.filter(requestContext);
+
+ ArgumentCaptor<URI> uriCaptor = ArgumentCaptor.forClass(URI.class);
+ Mockito.verify(requestContext).setRequestUri(Mockito.any(URI.class),
uriCaptor.capture());
+
+ URI capturedUri = uriCaptor.getValue();
+
Assert.assertEquals("/graphspaces/DEFAULT/schema/vertexlabels/person-label",
capturedUri.getPath());
+ }
+
+ /**
+ * Test URL encoded characters handling
+ */
+ @Test
+ public void testUrlEncoded_Characters() throws IOException {
+ // Path contains encoded space %20
+ setupUriInfo("/", "/schema/propertykeys/my%20key",
Arrays.asList("schema", "propertykeys", "my%20key"), null);
+
+ pathFilter.filter(requestContext);
+
+ ArgumentCaptor<URI> uriCaptor = ArgumentCaptor.forClass(URI.class);
+ Mockito.verify(requestContext).setRequestUri(Mockito.any(URI.class),
uriCaptor.capture());
+
+ URI capturedUri = uriCaptor.getValue();
+ // URI automatically decodes %20 to space
+ Assert.assertEquals("/graphspaces/DEFAULT/schema/propertykeys/my key",
capturedUri.getPath());
+ }
+
+ /**
+ * Test custom graph space configuration
+ */
+ @Test
+ public void testCustomGraphSpace_Configuration() throws IOException {
+ // Modify configuration to custom graph space
+ Configuration customConf = new PropertiesConfiguration();
+ customConf.setProperty(ServerOptions.PATH_GRAPH_SPACE.name(),
"CUSTOM_SPACE");
+ HugeConfig customConfig = new HugeConfig(customConf);
+
+ Provider<HugeConfig> customProvider = () -> customConfig;
+ injectProvider(this.pathFilter, customProvider);
+
+ setupUriInfo("/", "/graphs/test", Arrays.asList("graphs", "test"),
null);
+
+ pathFilter.filter(requestContext);
+
+ ArgumentCaptor<URI> uriCaptor = ArgumentCaptor.forClass(URI.class);
+ Mockito.verify(requestContext).setRequestUri(Mockito.any(URI.class),
uriCaptor.capture());
+
+ URI capturedUri = uriCaptor.getValue();
+ Assert.assertEquals("/graphspaces/CUSTOM_SPACE/graphs/test",
capturedUri.getPath());
+ }
+
+ /**
+ * Test deeply nested path
+ */
+ @Test
+ public void testDeeplyNested_Path() throws IOException {
+ setupUriInfo("/", "/graphs/hugegraph/traversers/shortestpath",
Arrays.asList("graphs", "hugegraph", "traversers", "shortestpath"), null);
+
+ pathFilter.filter(requestContext);
+
+ ArgumentCaptor<URI> uriCaptor = ArgumentCaptor.forClass(URI.class);
+ Mockito.verify(requestContext).setRequestUri(Mockito.any(URI.class),
uriCaptor.capture());
+
+ URI capturedUri = uriCaptor.getValue();
+
Assert.assertEquals("/graphspaces/DEFAULT/graphs/hugegraph/traversers/shortestpath",
capturedUri.getPath());
+ }
+
+ /**
+ * Test isWhiteAPI static method - single segment whitelist paths
+ * Note: PathFilter.isWhiteAPI() only checks the first segment in actual
usage
+ */
+ @Test
+ public void testIsWhiteAPI_AllWhiteListPaths() {
+ // Test single-segment whitelist entries (as used in
PathFilter.filter())
+ Assert.assertTrue(PathFilter.isWhiteAPI(""));
+ Assert.assertTrue(PathFilter.isWhiteAPI("apis"));
+ Assert.assertTrue(PathFilter.isWhiteAPI("metrics"));
+ Assert.assertTrue(PathFilter.isWhiteAPI("versions"));
+ Assert.assertTrue(PathFilter.isWhiteAPI("health"));
+ Assert.assertTrue(PathFilter.isWhiteAPI("gremlin"));
+ Assert.assertTrue(PathFilter.isWhiteAPI("auth"));
+ Assert.assertTrue(PathFilter.isWhiteAPI("hstore"));
+ Assert.assertTrue(PathFilter.isWhiteAPI("pd"));
+ Assert.assertTrue(PathFilter.isWhiteAPI("kafka"));
+ Assert.assertTrue(PathFilter.isWhiteAPI("openapi.json"));
+ }
+
+ /**
+ * Test isWhiteAPI static method - multi-segment strings
+ * Note: This tests the static method directly with multi-segment strings,
+ * but in actual usage, only the first segment is passed to isWhiteAPI()
+ */
+ @Test
+ public void testIsWhiteAPI_MultiSegmentStrings() {
+ // These are how multi-segment entries are stored in the whitelist set
+ Assert.assertTrue(PathFilter.isWhiteAPI("auth/users"));
+ Assert.assertTrue(PathFilter.isWhiteAPI("graphs/auth"));
+ Assert.assertTrue(PathFilter.isWhiteAPI("graphs/auth/users"));
+ }
+
+ /**
+ * Test isWhiteAPI static method - non-whitelist paths
+ */
+ @Test
+ public void testIsWhiteAPI_NonWhiteListPaths() {
+ Assert.assertFalse(PathFilter.isWhiteAPI("graphs"));
+ Assert.assertFalse(PathFilter.isWhiteAPI("schema"));
+ Assert.assertFalse(PathFilter.isWhiteAPI("vertices"));
+ Assert.assertFalse(PathFilter.isWhiteAPI("edges"));
+ Assert.assertFalse(PathFilter.isWhiteAPI("traversers"));
+ Assert.assertFalse(PathFilter.isWhiteAPI("tasks"));
+ Assert.assertFalse(PathFilter.isWhiteAPI("unknown"));
+ }
+}
+
Review Comment:
⚠️ **测试覆盖不完整**
当前测试用例缺少以下重要场景:
1. **边界情况**: 当配置的 `PATH_GRAPH_SPACE` 为空或 null 时的行为
2. **并发安全**: Provider 注入的配置在运行时更新时的行为
3. **错误路径**: segments 为空列表时的异常处理
4. **性能测试**: 大量请求下路径重写的性能影响
建议补充这些测试用例以提高代码健壮性。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
