imbajin commented on code in PR #2898:
URL:
https://github.com/apache/incubator-hugegraph/pull/2898#discussion_r2489991640
##########
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:
🧹 **代码风格 - 字符串拼接可以优化**
第107-110行的路径拼接可以使用更简洁的方式:
```suggestion
String path = String.format("%s%s/%s/%s",
uriInfo.getBaseUri().getPath(),
GRAPH_SPACE,
defaultPathSpace,
segments.stream()
.map(PathSegment::getPath)
.collect(Collectors.joining(DELIMITER)));
```
或使用 StringBuilder 来提高性能(如果这是高频调用路径)。
##########
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);
Review Comment:
🧹 **测试代码可以简化**
第84-90行的反射注入可以考虑使用 Mockito 的 `@InjectMocks` 和 `@Mock` 注解来简化:
```java
@Mock
private Provider<HugeConfig> configProvider;
@InjectMocks
private PathFilter pathFilter;
```
这样可以减少手动反射的代码,提高可读性。不过需要确认测试框架是否支持这种方式。
--
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]