-
Notifications
You must be signed in to change notification settings - Fork 571
Fix : SameNeighborsBatchAPI diff between inner and community Ver #2825
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,115 @@ | ||||||||
| /* | ||||||||
| * 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.api.traversers; | ||||||||
|
|
||||||||
| import static org.apache.hugegraph.traversal.algorithm.HugeTraverser.DEFAULT_ELEMENTS_LIMIT; | ||||||||
| import static org.apache.hugegraph.traversal.algorithm.HugeTraverser.DEFAULT_MAX_DEGREE; | ||||||||
|
|
||||||||
| import java.util.ArrayList; | ||||||||
| import java.util.List; | ||||||||
| import java.util.Set; | ||||||||
|
|
||||||||
| import org.apache.hugegraph.util.E; | ||||||||
| import org.apache.hugegraph.util.Log; | ||||||||
| import org.slf4j.Logger; | ||||||||
|
|
||||||||
| import org.apache.hugegraph.HugeGraph; | ||||||||
| import org.apache.hugegraph.api.API; | ||||||||
| import org.apache.hugegraph.api.graph.EdgeAPI; | ||||||||
| import org.apache.hugegraph.core.GraphManager; | ||||||||
| import org.apache.hugegraph.backend.id.Id; | ||||||||
| import org.apache.hugegraph.server.RestServer; | ||||||||
| import org.apache.hugegraph.structure.HugeVertex; | ||||||||
| import org.apache.hugegraph.traversal.algorithm.SameNeighborTraverser; | ||||||||
| import org.apache.hugegraph.type.define.Directions; | ||||||||
| import com.codahale.metrics.annotation.Timed; | ||||||||
| import com.fasterxml.jackson.annotation.JsonProperty; | ||||||||
| import com.fasterxml.jackson.databind.ObjectMapper; | ||||||||
|
|
||||||||
| import io.swagger.v3.oas.annotations.tags.Tag; | ||||||||
| import jakarta.inject.Singleton; | ||||||||
| import jakarta.ws.rs.POST; | ||||||||
| import jakarta.ws.rs.Path; | ||||||||
| import jakarta.ws.rs.PathParam; | ||||||||
| import jakarta.ws.rs.Produces; | ||||||||
| import jakarta.ws.rs.core.Context; | ||||||||
|
|
||||||||
| @Path("graphspaces/{graphspace}/graphs/{graph}/traversers/sameneighborsbatch") | ||||||||
| @Singleton | ||||||||
| @Tag(name = "SameNeighborsBatchAPI") | ||||||||
| public class SameNeighborsBatchAPI extends API { | ||||||||
|
Check warning on line 55 in hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java
|
||||||||
|
|
||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Line 56 creates a logger for Fix: private static final Logger LOG = Log.logger(SameNeighborsBatchAPI.class); |
||||||||
| private static final Logger LOG = Log.logger(RestServer.class); | ||||||||
|
Check warning on line 57 in hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java
|
||||||||
|
|
||||||||
| @POST | ||||||||
| @Timed | ||||||||
| @Produces(APPLICATION_JSON_WITH_CHARSET) | ||||||||
| public String sameNeighborsBatch(@Context GraphManager manager, | ||||||||
| @PathParam("graphspace") String graphSpace, | ||||||||
| @PathParam("graph") String graph, | ||||||||
| Request req) { | ||||||||
| LOG.debug("Graph [{}] get same neighbors batch, '{}'", graph, req.toString()); | ||||||||
|
Check warning on line 66 in hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java
|
||||||||
|
|
||||||||
| Directions dir = Directions.convert(EdgeAPI.parseDirection(req.direction)); | ||||||||
|
Check warning on line 68 in hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java
|
||||||||
|
|
||||||||
| ApiMeasurer measure = new ApiMeasurer(); | ||||||||
| HugeGraph g = graph(manager, graph); | ||||||||
| SameNeighborTraverser traverser = new SameNeighborTraverser(g); | ||||||||
|
Check warning on line 72 in hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java
|
||||||||
|
|
||||||||
| List<Set<Id>> result = new ArrayList<>(); | ||||||||
|
Check warning on line 74 in hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java
|
||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No null check for Add validation: E.checkArgumentNotNull(req.vertexList, "vertex_list cannot be null");
E.checkArgument(!req.vertexList.isEmpty(), "vertex_list cannot be empty");
for (List<Object> vertexPair : req.vertexList) {
E.checkArgumentNotNull(vertexPair, "vertex pair cannot be null");
E.checkArgument(vertexPair.size() == 2, "vertex_list size must be 2");
// ...
} |
||||||||
| for (List<Object> vertexPair : req.vertexList) { | ||||||||
| E.checkArgument(vertexPair.size() == 2, | ||||||||
| "vertex_list size must be 2"); | ||||||||
| Id sourceId = HugeVertex.getIdValue(vertexPair.get(0)); | ||||||||
| Id targetId = HugeVertex.getIdValue(vertexPair.get(1)); | ||||||||
| Set<Id> neighbors = traverser.sameNeighbors(sourceId, targetId, dir, | ||||||||
|
Check warning on line 80 in hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java
|
||||||||
| req.label, req.maxDegree, req.limit); | ||||||||
| result.add(neighbors); | ||||||||
| } | ||||||||
| measure.addIterCount(traverser.vertexIterCounter.get(), | ||||||||
| traverser.edgeIterCounter.get()); | ||||||||
|
Check warning on line 85 in hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java
|
||||||||
|
|
||||||||
| return manager.serializer(g, measure.measures()).writeList("same_neighbors", result); | ||||||||
|
Check warning on line 87 in hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java
|
||||||||
| } | ||||||||
|
|
||||||||
| private static class Request { | ||||||||
|
Check warning on line 90 in hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java
|
||||||||
| @JsonProperty("vertex_list") | ||||||||
| private List<List<Object>> vertexList; | ||||||||
| @JsonProperty("direction") | ||||||||
| private String direction; | ||||||||
| @JsonProperty("label") | ||||||||
| private String label; | ||||||||
| @JsonProperty("max_degree") | ||||||||
| public long maxDegree = Long.parseLong(DEFAULT_MAX_DEGREE); | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lines 97-98 use If
Current: public long maxDegree = Long.parseLong(DEFAULT_MAX_DEGREE);
public int limit = Integer.parseInt(DEFAULT_ELEMENTS_LIMIT);Better approach: public long maxDegree = DEFAULT_MAX_DEGREE_VALUE; // Define as long constant
public int limit = DEFAULT_ELEMENTS_LIMIT_VALUE; // Define as int constant |
||||||||
| @JsonProperty("limit") | ||||||||
| public int limit = Integer.parseInt(DEFAULT_ELEMENTS_LIMIT); | ||||||||
|
Check warning on line 100 in hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java
|
||||||||
|
|
||||||||
| @Override | ||||||||
| public String toString() { | ||||||||
| ObjectMapper om = new ObjectMapper(); | ||||||||
| String vertexStr = ""; | ||||||||
|
Check warning on line 105 in hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java
|
||||||||
| try { | ||||||||
| vertexStr = om.writeValueAsString(this.vertexList); | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lines 106-107 catch an exception but do nothing with it - not even logging. This is a bad practice that makes debugging impossible. Issues:
Fix: try {
vertexStr = om.writeValueAsString(this.vertexList);
} catch (Exception e) {
LOG.warn("Failed to serialize vertex list: {}", e.getMessage());
vertexStr = "<serialization failed>";
} |
||||||||
| } catch (Exception e) { | ||||||||
|
||||||||
| } catch (Exception e) { | |
| } catch (Exception e) { | |
| LOG.debug("Failed to serialize vertex list: {}", e.getMessage()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Minor: Typo in format string
Line 109 has an incomplete format string - missing closing brace } after limit=%d.
Fix:
return String.format("SameNeighborsBatchRequest{vertex=%s,direction=%s,label=%s," +
"max_degree=%d,limit=%d}", vertexStr, this.direction,
this.label, this.maxDegree, this.limit);Check warning on line 110 in hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java
Codecov / codecov/patch
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java#L107-L110
Added lines #L107 - L110 were not covered by tests
Copilot
AI
Oct 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing closing brace in format string. The string should end with } after limit=%d. Current output will be malformed like limit=10 instead of limit=10}.
| "max_degree=%d,limit=%d", vertexStr, this.direction, | |
| "max_degree=%d,limit=%d}", vertexStr, this.direction, |
Check warning on line 112 in hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java
Codecov / codecov/patch
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java#L112
Added line #L112 was not covered by tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import statements are not properly ordered. Standard Java convention and most projects require imports to be grouped and sorted:
Current order mixes Apache HugeGraph imports with third-party libraries (Codahale, Jackson, Swagger, Jakarta). This violates standard Java conventions.
Suggestion: Reorder imports to follow standard conventions: